Skip to content

Enumerate lint expectations using AttrId #130050

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

Merged
merged 2 commits into from
Sep 11, 2024
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
24 changes: 2 additions & 22 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ use std::thread::panicking;

use rustc_data_structures::fx::FxIndexMap;
use rustc_error_messages::{fluent_value_from_str_list_sep_by_and, FluentValue};
use rustc_lint_defs::{Applicability, LintExpectationId};
use rustc_lint_defs::Applicability;
use rustc_macros::{Decodable, Encodable};
use rustc_span::source_map::Spanned;
use rustc_span::symbol::Symbol;
use rustc_span::{AttrId, Span, DUMMY_SP};
use rustc_span::{Span, DUMMY_SP};
use tracing::debug;

use crate::snippet::Style;
Expand Down Expand Up @@ -354,26 +354,6 @@ impl DiagInner {
}
}

pub(crate) fn update_unstable_expectation_id(
&mut self,
unstable_to_stable: &FxIndexMap<AttrId, LintExpectationId>,
) {
if let Level::Expect(expectation_id) | Level::ForceWarning(Some(expectation_id)) =
&mut self.level
&& let LintExpectationId::Unstable { attr_id, lint_index } = *expectation_id
{
// 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 Some(stable_id) = unstable_to_stable.get(&attr_id) else {
panic!("{expectation_id:?} must have a matching stable id")
};

let mut stable_id = *stable_id;
stable_id.set_lint_index(lint_index);
*expectation_id = stable_id;
}
}

/// Indicates whether this diagnostic should show up in cargo's future breakage report.
pub(crate) fn has_future_breakage(&self) -> bool {
matches!(self.is_lint, Some(IsLint { has_future_breakage: true, .. }))
Expand Down
94 changes: 8 additions & 86 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ use rustc_macros::{Decodable, Encodable};
pub use rustc_span::fatal_error::{FatalError, FatalErrorMarker};
use rustc_span::source_map::SourceMap;
pub use rustc_span::ErrorGuaranteed;
use rustc_span::{AttrId, Loc, Span, DUMMY_SP};
use rustc_span::{Loc, Span, DUMMY_SP};
pub use snippet::Style;
// Used by external projects such as `rust-gpu`.
// See https://github.com/rust-lang/rust/pull/115393.
Expand Down Expand Up @@ -497,28 +497,18 @@ struct DiagCtxtInner {

future_breakage_diagnostics: Vec<DiagInner>,

/// The [`Self::unstable_expect_diagnostics`] should be empty when this struct is
/// dropped. However, it can have values if the compilation is stopped early
/// or is only partially executed. To avoid ICEs, like in rust#94953 we only
/// check if [`Self::unstable_expect_diagnostics`] is empty, if the expectation ids
/// have been converted.
check_unstable_expect_diagnostics: bool,

/// Expected [`DiagInner`][struct@diagnostic::DiagInner]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 [`DiagInner`][struct@diagnostic::DiagInner]s
/// are submitted for storage and added to the list of fulfilled expectations.
unstable_expect_diagnostics: Vec<DiagInner>,

/// 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.
///
/// Emitting expectations after having stolen this field can happen. In particular, an
/// `#[expect(warnings)]` can easily make the `UNFULFILLED_LINT_EXPECTATIONS` lint expect
/// itself. To avoid needless complexity in this corner case, we tolerate failing to track
/// those expectations.
///
/// [RFC-2383]: https://rust-lang.github.io/rfcs/2383-lint-reasons.html
fulfilled_expectations: FxHashSet<LintExpectationId>,
fulfilled_expectations: FxIndexSet<LintExpectationId>,

/// The file where the ICE information is stored. This allows delayed_span_bug backtraces to be
/// stored along side the main panic backtrace.
Expand Down Expand Up @@ -605,13 +595,6 @@ impl Drop for DiagCtxtInner {
);
}
}

if self.check_unstable_expect_diagnostics {
assert!(
self.unstable_expect_diagnostics.is_empty(),
"all diagnostics with unstable expectations should have been converted",
);
}
}
}

Expand Down Expand Up @@ -740,8 +723,6 @@ impl DiagCtxt {
emitted_diagnostics,
stashed_diagnostics,
future_breakage_diagnostics,
check_unstable_expect_diagnostics,
unstable_expect_diagnostics,
fulfilled_expectations,
ice_file: _,
} = inner.deref_mut();
Expand All @@ -761,8 +742,6 @@ impl DiagCtxt {
*emitted_diagnostics = Default::default();
*stashed_diagnostics = Default::default();
*future_breakage_diagnostics = Default::default();
*check_unstable_expect_diagnostics = false;
*unstable_expect_diagnostics = Default::default();
*fulfilled_expectations = Default::default();
}

Expand Down Expand Up @@ -1094,44 +1073,10 @@ impl<'a> DiagCtxtHandle<'a> {
inner.emitter.emit_unused_externs(lint_level, unused_externs)
}

pub fn update_unstable_expectation_id(
&self,
unstable_to_stable: FxIndexMap<AttrId, LintExpectationId>,
) {
let mut inner = self.inner.borrow_mut();
let diags = std::mem::take(&mut inner.unstable_expect_diagnostics);
inner.check_unstable_expect_diagnostics = true;

if !diags.is_empty() {
inner.suppressed_expected_diag = true;
for mut diag in diags.into_iter() {
diag.update_unstable_expectation_id(&unstable_to_stable);

// Here the diagnostic is given back to `emit_diagnostic` where it was first
// intercepted. Now it should be processed as usual, since the unstable expectation
// id is now stable.
inner.emit_diagnostic(diag, self.tainted_with_errors);
}
}

inner
.stashed_diagnostics
.values_mut()
.for_each(|(diag, _guar)| diag.update_unstable_expectation_id(&unstable_to_stable));
inner
.future_breakage_diagnostics
.iter_mut()
.for_each(|diag| diag.update_unstable_expectation_id(&unstable_to_stable));
}

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

Expand Down Expand Up @@ -1440,8 +1385,6 @@ impl DiagCtxtInner {
emitted_diagnostics: Default::default(),
stashed_diagnostics: Default::default(),
future_breakage_diagnostics: Vec::new(),
check_unstable_expect_diagnostics: false,
unstable_expect_diagnostics: Vec::new(),
fulfilled_expectations: Default::default(),
ice_file: None,
}
Expand Down Expand Up @@ -1471,24 +1414,6 @@ impl DiagCtxtInner {
mut diagnostic: DiagInner,
taint: Option<&Cell<Option<ErrorGuaranteed>>>,
) -> Option<ErrorGuaranteed> {
match diagnostic.level {
Expect(expect_id) | ForceWarning(Some(expect_id)) => {
// 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 LintExpectationId::Unstable { .. } = expect_id {
// We don't call TRACK_DIAGNOSTIC because we wait for the
// unstable ID to be updated, whereupon the diagnostic will be
// passed into this method again.
self.unstable_expect_diagnostics.push(diagnostic);
return None;
}
// Continue through to the `Expect`/`ForceWarning` case below.
}
_ => {}
}

if diagnostic.has_future_breakage() {
// Future breakages aren't emitted if they're `Level::Allow` or
// `Level::Expect`, but they still need to be constructed and
Expand Down Expand Up @@ -1564,9 +1489,6 @@ impl DiagCtxtInner {
return None;
}
Expect(expect_id) | ForceWarning(Some(expect_id)) => {
if let LintExpectationId::Unstable { .. } = expect_id {
unreachable!(); // this case was handled at the top of this function
}
self.fulfilled_expectations.insert(expect_id);
if let Expect(_) = diagnostic.level {
// Nothing emitted here for expected lints.
Expand Down
90 changes: 39 additions & 51 deletions compiler/rustc_lint/src/expect.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use rustc_data_structures::fx::FxIndexMap;
use rustc_hir::{HirId, CRATE_OWNER_ID};
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::CRATE_OWNER_ID;
use rustc_middle::lint::LintExpectation;
use rustc_middle::query::Providers;
use rustc_middle::ty::TyCtxt;
use rustc_session::lint::builtin::UNFULFILLED_LINT_EXPECTATIONS;
use rustc_session::lint::{Level, LintExpectationId};
use rustc_session::lint::LintExpectationId;
use rustc_span::Symbol;

use crate::lints::{Expectation, ExpectationNote};
Expand All @@ -17,68 +17,56 @@ fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExp
let krate = tcx.hir_crate_items(());

let mut expectations = Vec::new();
let mut unstable_to_stable_ids = FxIndexMap::default();

let mut record_stable = |attr_id, hir_id, attr_index| {
let expect_id = LintExpectationId::Stable { hir_id, attr_index, lint_index: None };
unstable_to_stable_ids.entry(attr_id).or_insert(expect_id);
};
let mut push_expectations = |owner| {
for owner in std::iter::once(CRATE_OWNER_ID).chain(krate.owners()) {
let lints = tcx.shallow_lint_levels_on(owner);
if lints.expectations.is_empty() {
return;
}

expectations.extend_from_slice(&lints.expectations);

let attrs = tcx.hir_attrs(owner);
for &(local_id, attrs) in attrs.map.iter() {
// Some attributes appear multiple times in HIR, to ensure they are correctly taken
// into account where they matter. This means we cannot just associate the AttrId to
// the first HirId where we see it, but need to check it actually appears in a lint
// level.
// FIXME(cjgillot): Can this cause an attribute to appear in multiple expectation ids?
if !lints.specs.contains_key(&local_id) {
continue;
}
for (attr_index, attr) in attrs.iter().enumerate() {
let Some(Level::Expect(_)) = Level::from_attr(attr) else { continue };
record_stable(attr.id, HirId { owner, local_id }, attr_index.try_into().unwrap());
}
}
};

push_expectations(CRATE_OWNER_ID);
for owner in krate.owners() {
push_expectations(owner);
}

tcx.dcx().update_unstable_expectation_id(unstable_to_stable_ids);
expectations
}

fn check_expectations(tcx: TyCtxt<'_>, tool_filter: Option<Symbol>) {
let lint_expectations = tcx.lint_expectations(());
let fulfilled_expectations = tcx.dcx().steal_fulfilled_expectation_ids();

for (id, expectation) in lint_expectations {
// This check will always be true, since `lint_expectations` only
// holds stable ids
if let LintExpectationId::Stable { hir_id, .. } = id {
if !fulfilled_expectations.contains(id)
&& tool_filter.map_or(true, |filter| expectation.lint_tool == Some(filter))
{
let rationale = expectation.reason.map(|rationale| ExpectationNote { rationale });
let note = expectation.is_unfulfilled_lint_expectations;
tcx.emit_node_span_lint(
UNFULFILLED_LINT_EXPECTATIONS,
*hir_id,
expectation.emission_span,
Expectation { rationale, note },
);
// Turn a `LintExpectationId` into a `(AttrId, lint_index)` pair.
let canonicalize_id = |expect_id: &LintExpectationId| {
match *expect_id {
LintExpectationId::Unstable { attr_id, lint_index: Some(lint_index) } => {
(attr_id, lint_index)
}
LintExpectationId::Stable { hir_id, attr_index, lint_index: Some(lint_index) } => {
// We are an `eval_always` query, so looking at the attribute's `AttrId` is ok.
let attr_id = tcx.hir().attrs(hir_id)[attr_index as usize].id;
(attr_id, lint_index)
}
} else {
_ => panic!("fulfilled expectations must have a lint index"),
}
};

let fulfilled_expectations: FxHashSet<_> =
fulfilled_expectations.iter().map(canonicalize_id).collect();

for (expect_id, expectation) in lint_expectations {
// This check will always be true, since `lint_expectations` only holds stable ids
let LintExpectationId::Stable { hir_id, .. } = expect_id else {
unreachable!("at this stage all `LintExpectationId`s are stable");
};

let expect_id = canonicalize_id(expect_id);

if !fulfilled_expectations.contains(&expect_id)
&& tool_filter.map_or(true, |filter| expectation.lint_tool == Some(filter))
{
let rationale = expectation.reason.map(|rationale| ExpectationNote { rationale });
let note = expectation.is_unfulfilled_lint_expectations;
tcx.emit_node_span_lint(
UNFULFILLED_LINT_EXPECTATIONS,
*hir_id,
expectation.emission_span,
Expectation { rationale, note },
);
}
}
}
9 changes: 9 additions & 0 deletions tests/ui/lint/expect-unused-imports.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@ check-pass

#![deny(unused_imports)]
#![deny(unfulfilled_lint_expectations)]

#[expect(unused_imports)]
use std::{io, fs};

fn main() {}
6 changes: 6 additions & 0 deletions tests/ui/lint/rfc-2383-lint-reason/expect_warnings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//@ check-pass

#![expect(warnings)]

#[expect(unused)]
fn main() {}
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
warning: denote infinite loops with `loop { ... }`
--> $DIR/force_warn_expected_lints_fulfilled.rs:8:5
|
LL | while true {
| ^^^^^^^^^^ help: use `loop`
|
= note: requested on the command line with `--force-warn while-true`

warning: unused variable: `x`
--> $DIR/force_warn_expected_lints_fulfilled.rs:18:9
|
Expand Down Expand Up @@ -28,13 +36,5 @@ warning: unused variable: `this_should_fulfill_the_expectation`
LL | let this_should_fulfill_the_expectation = "The `#[allow]` has no power here";
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_this_should_fulfill_the_expectation`

warning: denote infinite loops with `loop { ... }`
--> $DIR/force_warn_expected_lints_fulfilled.rs:8:5
|
LL | while true {
| ^^^^^^^^^^ help: use `loop`
|
= note: requested on the command line with `--force-warn while-true`

warning: 5 warnings emitted

Loading