Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of the expect attribute (RFC 2383) #87835

Merged
merged 13 commits into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3882,6 +3882,7 @@ version = "0.0.0"
dependencies = [
"rustc_ast",
"rustc_data_structures",
"rustc_hir",
"rustc_macros",
"rustc_serialize",
"rustc_span",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ fn annotation_type_for_level(level: Level) -> AnnotationType {
// FIXME(#59346): Not sure how to map this level
Level::FailureNote => AnnotationType::Error,
Level::Allow => panic!("Should not call with Allow"),
Level::Expect(_) => panic!("Should not call with Expect"),
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl Diagnostic {
| Level::Error { .. }
| Level::FailureNote => true,

Level::Warning | Level::Note | Level::Help | Level::Allow => false,
Level::Warning | Level::Note | Level::Help | Level::Allow | Level::Expect(_) => false,
}
}

Expand Down
93 changes: 90 additions & 3 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ extern crate tracing;

pub use emitter::ColorConfig;

use rustc_lint_defs::LintExpectationId;
use Level::*;

use emitter::{is_case_difference, Emitter, EmitterWriter};
use registry::Registry;
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
use rustc_data_structures::stable_hasher::StableHasher;
use rustc_data_structures::sync::{self, Lock, Lrc};
use rustc_data_structures::AtomicRef;
Expand Down Expand Up @@ -450,6 +451,22 @@ struct HandlerInner {
deduplicated_warn_count: usize,

future_breakage_diagnostics: Vec<Diagnostic>,

/// Expected [`Diagnostic`]s store a [`LintExpectationId`] as part of
/// the lint level. [`LintExpectationId`]s created early during the compilation
/// (before `HirId`s have been defined) are not stable and can therefore not be
/// stored on disk. This buffer stores these diagnostics until the ID has been
/// replaced by a stable [`LintExpectationId`]. The [`Diagnostic`]s are the
/// submitted for storage and added to the list of fulfilled expectations.
unstable_expect_diagnostics: Vec<Diagnostic>,
xFrednet marked this conversation as resolved.
Show resolved Hide resolved

/// expected diagnostic will have the level `Expect` which additionally
/// carries the [`LintExpectationId`] of the expectation that can be
/// marked as fulfilled. This is a collection of all [`LintExpectationId`]s
/// that have been marked as fulfilled this way.
///
/// [RFC-2383]: https://rust-lang.github.io/rfcs/2383-lint-reasons.html
fulfilled_expectations: FxHashSet<LintExpectationId>,
}

/// A key denoting where from a diagnostic was stashed.
Expand Down Expand Up @@ -570,6 +587,8 @@ impl Handler {
emitted_diagnostics: Default::default(),
stashed_diagnostics: Default::default(),
future_breakage_diagnostics: Vec::new(),
unstable_expect_diagnostics: Vec::new(),
fulfilled_expectations: Default::default(),
}),
}
}
Expand Down Expand Up @@ -677,6 +696,11 @@ impl Handler {
DiagnosticBuilder::new(self, Level::Allow, msg)
}

/// Construct a builder at the `Expect` level with the `msg`.
pub fn struct_expect(&self, msg: &str, id: LintExpectationId) -> DiagnosticBuilder<'_, ()> {
DiagnosticBuilder::new(self, Level::Expect(id), msg)
}

/// Construct a builder at the `Error` level at the given `span` and with the `msg`.
pub fn struct_span_err(
&self,
Expand Down Expand Up @@ -906,6 +930,48 @@ impl Handler {
pub fn emit_unused_externs(&self, lint_level: &str, unused_externs: &[&str]) {
self.inner.borrow_mut().emit_unused_externs(lint_level, unused_externs)
}

pub fn update_unstable_expectation_id(
&self,
unstable_to_stable: &FxHashMap<LintExpectationId, LintExpectationId>,
) {
let diags = std::mem::take(&mut self.inner.borrow_mut().unstable_expect_diagnostics);
if diags.is_empty() {
return;
}

let mut inner = self.inner.borrow_mut();
for mut diag in diags.into_iter() {
let mut unstable_id = diag
.level
.get_expectation_id()
.expect("all diagnostics inside `unstable_expect_diagnostics` must have a `LintExpectationId`");

// The unstable to stable map only maps the unstable `AttrId` to a stable `HirId` with an attribute index.
// The lint index inside the attribute is manually transferred here.
let lint_index = unstable_id.get_lint_index();
unstable_id.set_lint_index(None);
let mut stable_id = *unstable_to_stable
.get(&unstable_id)
.expect("each unstable `LintExpectationId` must have a matching stable id");

stable_id.set_lint_index(lint_index);
diag.level = Level::Expect(stable_id);
inner.fulfilled_expectations.insert(stable_id);

(*TRACK_DIAGNOSTICS)(&diag);
}
}

/// This methods steals all [`LintExpectationId`]s that are stored inside
/// [`HandlerInner`] and indicate that the linked expectation has been fulfilled.
pub fn steal_fulfilled_expectation_ids(&self) -> FxHashSet<LintExpectationId> {
assert!(
self.inner.borrow().unstable_expect_diagnostics.is_empty(),
"`HandlerInner::unstable_expect_diagnostics` should be empty at this point",
);
std::mem::take(&mut self.inner.borrow_mut().fulfilled_expectations)
}
}

impl HandlerInner {
Expand Down Expand Up @@ -951,9 +1017,21 @@ impl HandlerInner {
return;
}

// The `LintExpectationId` can be stable or unstable depending on when it was created.
// Diagnostics created before the definition of `HirId`s are unstable and can not yet
// be stored. Instead, they are buffered until the `LintExpectationId` is replaced by
// a stable one by the `LintLevelsBuilder`.
if let Level::Expect(LintExpectationId::Unstable { .. }) = diagnostic.level {
self.unstable_expect_diagnostics.push(diagnostic.clone());
return;
}

(*TRACK_DIAGNOSTICS)(diagnostic);

if diagnostic.level == Allow {
if let Level::Expect(expectation_id) = diagnostic.level {
self.fulfilled_expectations.insert(expectation_id);
return;
} else if diagnostic.level == Allow {
return;
}

Expand Down Expand Up @@ -1250,6 +1328,7 @@ pub enum Level {
Help,
FailureNote,
Allow,
Expect(LintExpectationId),
}

impl fmt::Display for Level {
Expand All @@ -1275,7 +1354,7 @@ impl Level {
spec.set_fg(Some(Color::Cyan)).set_intense(true);
}
FailureNote => {}
Allow => unreachable!(),
Allow | Expect(_) => unreachable!(),
}
spec
}
Expand All @@ -1289,12 +1368,20 @@ impl Level {
Help => "help",
FailureNote => "failure-note",
Allow => panic!("Shouldn't call on allowed error"),
Expect(_) => panic!("Shouldn't call on expected error"),
}
}

pub fn is_failure_note(&self) -> bool {
matches!(*self, FailureNote)
}

pub fn get_expectation_id(&self) -> Option<LintExpectationId> {
match self {
Level::Expect(id) => Some(*id),
_ => None,
}
}
}

// FIXME(eddyb) this doesn't belong here AFAICT, should be moved to callsite.
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
ungated!(
allow, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk
),
gated!(
expect, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk,
lint_reasons, experimental!(expect)
),
ungated!(
forbid, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk
),
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ struct LintGroup {
depr: Option<LintAlias>,
}

#[derive(Debug)]
pub enum CheckLintNameResult<'a> {
Ok(&'a [LintId]),
/// Lint doesn't exist. Potentially contains a suggestion for a correct lint name.
Expand Down Expand Up @@ -377,6 +378,9 @@ impl LintStore {
Level::ForceWarn => "--force-warn",
Level::Deny => "-D",
Level::Forbid => "-F",
Level::Expect(_) => {
unreachable!("lints with the level of `expect` should not run this code");
}
},
lint_name
);
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_lint/src/early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ impl<'a, T: EarlyLintPass> EarlyContextAndPass<'a, T> {
F: FnOnce(&mut Self),
{
let is_crate_node = id == ast::CRATE_NODE_ID;
let push = self.context.builder.push(attrs, is_crate_node);
let push = self.context.builder.push(attrs, is_crate_node, None);

self.check_id(id);
self.enter_attrs(attrs);
f(self);
Expand Down
49 changes: 49 additions & 0 deletions compiler/rustc_lint/src/expect.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
use crate::builtin;
use rustc_hir::HirId;
use rustc_middle::{lint::LintExpectation, ty::TyCtxt};
use rustc_session::lint::LintExpectationId;
use rustc_span::symbol::sym;

pub fn check_expectations(tcx: TyCtxt<'_>) {
if !tcx.sess.features_untracked().enabled(sym::lint_reasons) {
xFrednet marked this conversation as resolved.
Show resolved Hide resolved
return;
}

let fulfilled_expectations = tcx.sess.diagnostic().steal_fulfilled_expectation_ids();
let lint_expectations = &tcx.lint_levels(()).lint_expectations;

for (id, expectation) in lint_expectations {
if !fulfilled_expectations.contains(id) {
// This check will always be true, since `lint_expectations` only
xFrednet marked this conversation as resolved.
Show resolved Hide resolved
// holds stable ids
if let LintExpectationId::Stable { hir_id, .. } = id {
emit_unfulfilled_expectation_lint(tcx, *hir_id, expectation);
} else {
unreachable!("at this stage all `LintExpectationId`s are stable");
}
}
}
}

fn emit_unfulfilled_expectation_lint(
tcx: TyCtxt<'_>,
hir_id: HirId,
expectation: &LintExpectation,
) {
// FIXME: The current implementation doesn't cover cases where the
// `unfulfilled_lint_expectations` is actually expected by another lint
// expectation. This can be added here by checking the lint level and
// retrieving the `LintExpectationId` if it was expected.
tcx.struct_span_lint_hir(
builtin::UNFULFILLED_LINT_EXPECTATIONS,
hir_id,
expectation.emission_span,
|diag| {
let mut diag = diag.build("this lint expectation is unfulfilled");
if let Some(rationale) = expectation.reason {
diag.note(&rationale.as_str());
}
diag.emit();
},
);
}
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,4 +503,7 @@ pub fn check_crate<'tcx, T: LateLintPass<'tcx>>(
});
},
);

// This check has to be run after all lints are done processing for this crate
tcx.sess.time("check_lint_expectations", || crate::expect::check_expectations(tcx));
}
Loading