mirror of https://github.com/rust-lang/rust.git
Auto merge of #130050 - cjgillot:expect-attr-id, r=fee1-dead
Enumerate lint expectations using AttrId This PR implements the idea I outlined in https://github.com/rust-lang/rust/issues/127884#issuecomment-2240338547 We can uniquely identify a lint expectation `#[expect(lint0, lint1...)]` using the `AttrId` and the index of the lint inside the attribute. This PR uses this property in `check_expectations`. In addition, this PR stops stashing expected diagnostics to wait for the unstable -> stable `LintExpectationId` mapping: if the lint is emitted with an unstable attribute, it must have been emitted by an `eval_always` query (like inside the resolver), so won't be loaded from cache. Decoding an `AttrId` from the on-disk cache ICEs, so we have no risk of accidentally checking an expectation. Fixes https://github.com/rust-lang/rust/issues/127884 cc `@xFrednet`
This commit is contained in:
bors
commit
4c5fc2c334
|
|
@ -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;
|
||||
|
|
@ -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, .. }))
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -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();
|
||||
|
|
@ -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();
|
||||
}
|
||||
|
||||
|
|
@ -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)
|
||||
}
|
||||
|
||||
|
|
@ -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,
|
||||
}
|
||||
|
|
@ -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
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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};
|
||||
|
|
@ -17,43 +17,12 @@ 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
|
||||
}
|
||||
|
||||
|
|
@ -61,24 +30,43 @@ 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)
|
||||
}
|
||||
} else {
|
||||
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)
|
||||
}
|
||||
_ => 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 },
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,9 @@
|
|||
//@ check-pass
|
||||
|
||||
#![deny(unused_imports)]
|
||||
#![deny(unfulfilled_lint_expectations)]
|
||||
|
||||
#[expect(unused_imports)]
|
||||
use std::{io, fs};
|
||||
|
||||
fn main() {}
|
||||
|
|
@ -0,0 +1,6 @@
|
|||
//@ check-pass
|
||||
|
||||
#![expect(warnings)]
|
||||
|
||||
#[expect(unused)]
|
||||
fn main() {}
|
||||
|
|
@ -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
|
||||
|
|
||||
|
|
@ -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…
Reference in New Issue