From 4928b22fa8a429f128d5992dc273a39be9ac7a37 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 4 Jul 2024 06:31:07 +0000 Subject: [PATCH 1/4] Use AttrId key for unstable<->stable expectation map. --- compiler/rustc_errors/src/diagnostic.rs | 19 +++++++------------ compiler/rustc_errors/src/lib.rs | 10 +++++----- compiler/rustc_lint/src/levels.rs | 9 ++++----- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 3303e4ee752..7e4698a94a9 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -12,7 +12,7 @@ use rustc_lint_defs::{Applicability, LintExpectationId}; use rustc_macros::{Decodable, Encodable}; use rustc_span::source_map::Spanned; use rustc_span::symbol::Symbol; -use rustc_span::{Span, DUMMY_SP}; +use rustc_span::{AttrId, Span, DUMMY_SP}; use tracing::debug; use crate::snippet::Style; @@ -356,24 +356,19 @@ impl DiagInner { pub(crate) fn update_unstable_expectation_id( &mut self, - unstable_to_stable: &FxIndexMap, + unstable_to_stable: &FxIndexMap, ) { if let Level::Expect(expectation_id) | Level::ForceWarning(Some(expectation_id)) = &mut self.level + && let LintExpectationId::Unstable { attr_id, lint_index } = *expectation_id { - if expectation_id.is_stable() { - return; - } - // 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 = expectation_id.get_lint_index(); - expectation_id.set_lint_index(None); - let mut stable_id = unstable_to_stable - .get(expectation_id) - .expect("each unstable `LintExpectationId` must have a matching stable id") - .normalize(); + 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.normalize(); stable_id.set_lint_index(lint_index); *expectation_id = stable_id; } diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 99ee8fb17d7..967e64c2d9f 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -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::{Loc, Span, DUMMY_SP}; +use rustc_span::{AttrId, 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. @@ -1096,7 +1096,7 @@ impl<'a> DiagCtxtHandle<'a> { pub fn update_unstable_expectation_id( &self, - unstable_to_stable: &FxIndexMap, + unstable_to_stable: FxIndexMap, ) { let mut inner = self.inner.borrow_mut(); let diags = std::mem::take(&mut inner.unstable_expect_diagnostics); @@ -1105,7 +1105,7 @@ impl<'a> DiagCtxtHandle<'a> { if !diags.is_empty() { inner.suppressed_expected_diag = true; for mut diag in diags.into_iter() { - diag.update_unstable_expectation_id(unstable_to_stable); + 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 @@ -1117,11 +1117,11 @@ impl<'a> DiagCtxtHandle<'a> { inner .stashed_diagnostics .values_mut() - .for_each(|(diag, _guar)| diag.update_unstable_expectation_id(unstable_to_stable)); + .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)); + .for_each(|diag| diag.update_unstable_expectation_id(&unstable_to_stable)); } /// This methods steals all [`LintExpectationId`]s that are stored inside diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index 07ac63ec96c..aa7042ff625 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -20,7 +20,7 @@ use rustc_session::lint::builtin::{ use rustc_session::lint::{Level, Lint, LintExpectationId, LintId}; use rustc_session::Session; use rustc_span::symbol::{sym, Symbol}; -use rustc_span::{Span, DUMMY_SP}; +use rustc_span::{AttrId, Span, DUMMY_SP}; use tracing::{debug, instrument}; use {rustc_ast as ast, rustc_hir as hir}; @@ -138,7 +138,7 @@ fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExp builder.add_id(hir::CRATE_HIR_ID); tcx.hir().walk_toplevel_module(&mut builder); - tcx.dcx().update_unstable_expectation_id(&builder.provider.unstable_to_stable_ids); + tcx.dcx().update_unstable_expectation_id(builder.provider.unstable_to_stable_ids); builder.provider.expectations } @@ -252,7 +252,7 @@ struct QueryMapExpectationsWrapper<'tcx> { /// Level map for `cur`. specs: ShallowLintLevelMap, expectations: Vec<(LintExpectationId, LintExpectation)>, - unstable_to_stable_ids: FxIndexMap, + unstable_to_stable_ids: FxIndexMap, /// Empty hash map to simplify code. empty: FxIndexMap, } @@ -274,9 +274,8 @@ impl LintLevelsProvider for QueryMapExpectationsWrapper<'_> { else { bug!("unstable expectation id should already be mapped") }; - let key = LintExpectationId::Unstable { attr_id, lint_index: None }; - self.unstable_to_stable_ids.entry(key).or_insert(LintExpectationId::Stable { + self.unstable_to_stable_ids.entry(attr_id).or_insert(LintExpectationId::Stable { hir_id, attr_index, lint_index: None, From 111b0a97b4c849d7abac396fc4c6dd6a159c8560 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 4 Jul 2024 09:55:42 +0000 Subject: [PATCH 2/4] Rewrite lint_expectations in a single pass. --- compiler/rustc_lint/src/expect.rs | 54 ++++++- compiler/rustc_lint/src/levels.rs | 150 +----------------- compiler/rustc_middle/src/lint.rs | 3 +- tests/ui/error-codes/E0602.stderr | 7 +- tests/ui/lint/cli-unknown-force-warn.stderr | 7 +- .../ui/lint/lint-removed-cmdline-deny.stderr | 7 +- tests/ui/lint/lint-removed-cmdline.stderr | 7 +- .../ui/lint/lint-renamed-cmdline-deny.stderr | 8 +- tests/ui/lint/lint-renamed-cmdline.stderr | 8 +- .../ui/lint/lint-unexported-no-mangle.stderr | 12 +- .../lint-unknown-lint-cmdline-deny.stderr | 13 +- .../ui/lint/lint-unknown-lint-cmdline.stderr | 13 +- 12 files changed, 67 insertions(+), 222 deletions(-) diff --git a/compiler/rustc_lint/src/expect.rs b/compiler/rustc_lint/src/expect.rs index 42b33f9882d..d9fab1d00aa 100644 --- a/compiler/rustc_lint/src/expect.rs +++ b/compiler/rustc_lint/src/expect.rs @@ -1,21 +1,67 @@ +use rustc_data_structures::fx::FxIndexMap; +use rustc_hir::{HirId, 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::LintExpectationId; +use rustc_session::lint::{Level, LintExpectationId}; use rustc_span::Symbol; use crate::lints::{Expectation, ExpectationNote}; pub(crate) fn provide(providers: &mut Providers) { - *providers = Providers { check_expectations, ..*providers }; + *providers = Providers { lint_expectations, check_expectations, ..*providers }; +} + +fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExpectation)> { + 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, attr_id: None }; + unstable_to_stable_ids.entry(attr_id).or_insert(expect_id); + }; + let mut push_expectations = |owner| { + 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) { let lint_expectations = tcx.lint_expectations(()); let fulfilled_expectations = tcx.dcx().steal_fulfilled_expectation_ids(); - tracing::debug!(?lint_expectations, ?fulfilled_expectations); - for (id, expectation) in lint_expectations { // This check will always be true, since `lint_expectations` only // holds stable ids diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index aa7042ff625..dfbc61d50e8 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -20,7 +20,7 @@ use rustc_session::lint::builtin::{ use rustc_session::lint::{Level, Lint, LintExpectationId, LintId}; use rustc_session::Session; use rustc_span::symbol::{sym, Symbol}; -use rustc_span::{AttrId, Span, DUMMY_SP}; +use rustc_span::{Span, DUMMY_SP}; use tracing::{debug, instrument}; use {rustc_ast as ast, rustc_hir as hir}; @@ -115,34 +115,6 @@ impl LintLevelSets { } } -fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExpectation)> { - let store = unerased_lint_store(tcx.sess); - - let mut builder = LintLevelsBuilder { - sess: tcx.sess, - features: tcx.features(), - provider: QueryMapExpectationsWrapper { - tcx, - cur: hir::CRATE_HIR_ID, - specs: ShallowLintLevelMap::default(), - expectations: Vec::new(), - unstable_to_stable_ids: FxIndexMap::default(), - empty: FxIndexMap::default(), - }, - lint_added_lints: false, - store, - registered_tools: tcx.registered_tools(()), - }; - - builder.add_command_line(); - builder.add_id(hir::CRATE_HIR_ID); - tcx.hir().walk_toplevel_module(&mut builder); - - tcx.dcx().update_unstable_expectation_id(builder.provider.unstable_to_stable_ids); - - builder.provider.expectations -} - #[instrument(level = "trace", skip(tcx), ret)] fn shallow_lint_levels_on(tcx: TyCtxt<'_>, owner: hir::OwnerId) -> ShallowLintLevelMap { let store = unerased_lint_store(tcx.sess); @@ -207,7 +179,7 @@ pub trait LintLevelsProvider { fn current_specs(&self) -> &FxIndexMap; fn insert(&mut self, id: LintId, lvl: LevelAndSource); fn get_lint_level(&self, lint: &'static Lint, sess: &Session) -> LevelAndSource; - fn push_expectation(&mut self, _id: LintExpectationId, _expectation: LintExpectation) {} + fn push_expectation(&mut self, id: LintExpectationId, expectation: LintExpectation); } impl LintLevelsProvider for TopDown { @@ -222,6 +194,8 @@ impl LintLevelsProvider for TopDown { fn get_lint_level(&self, lint: &'static Lint, sess: &Session) -> LevelAndSource { self.sets.get_lint_level(lint, self.cur, Some(self.current_specs()), sess) } + + fn push_expectation(&mut self, _: LintExpectationId, _: LintExpectation) {} } struct LintLevelQueryMap<'tcx> { @@ -243,46 +217,8 @@ impl LintLevelsProvider for LintLevelQueryMap<'_> { fn get_lint_level(&self, lint: &'static Lint, _: &Session) -> LevelAndSource { self.specs.lint_level_id_at_node(self.tcx, LintId::of(lint), self.cur) } -} - -struct QueryMapExpectationsWrapper<'tcx> { - tcx: TyCtxt<'tcx>, - /// HirId of the currently investigated element. - cur: HirId, - /// Level map for `cur`. - specs: ShallowLintLevelMap, - expectations: Vec<(LintExpectationId, LintExpectation)>, - unstable_to_stable_ids: FxIndexMap, - /// Empty hash map to simplify code. - empty: FxIndexMap, -} - -impl LintLevelsProvider for QueryMapExpectationsWrapper<'_> { - fn current_specs(&self) -> &FxIndexMap { - self.specs.specs.get(&self.cur.local_id).unwrap_or(&self.empty) - } - fn insert(&mut self, id: LintId, lvl: LevelAndSource) { - self.specs.specs.get_mut_or_insert_default(self.cur.local_id).insert(id, lvl); - } - fn get_lint_level(&self, lint: &'static Lint, _: &Session) -> LevelAndSource { - // We cannot use `tcx.lint_level_at_node` because we want to know in which order the - // attributes have been inserted, in particular whether an `expect` follows a `forbid`. - self.specs.lint_level_id_at_node(self.tcx, LintId::of(lint), self.cur) - } fn push_expectation(&mut self, id: LintExpectationId, expectation: LintExpectation) { - let LintExpectationId::Stable { attr_id: Some(attr_id), hir_id, attr_index, .. } = id - else { - bug!("unstable expectation id should already be mapped") - }; - - self.unstable_to_stable_ids.entry(attr_id).or_insert(LintExpectationId::Stable { - hir_id, - attr_index, - lint_index: None, - attr_id: None, - }); - - self.expectations.push((id.normalize(), expectation)); + self.specs.expectations.push((id.normalize(), expectation)) } } @@ -367,80 +303,6 @@ impl<'tcx> Visitor<'tcx> for LintLevelsBuilder<'_, LintLevelQueryMap<'tcx>> { } } -impl<'tcx> LintLevelsBuilder<'_, QueryMapExpectationsWrapper<'tcx>> { - fn add_id(&mut self, hir_id: HirId) { - // Change both the `HirId` and the associated specs. - self.provider.cur = hir_id; - self.provider.specs.specs.clear(); - self.add(self.provider.tcx.hir().attrs(hir_id), hir_id == hir::CRATE_HIR_ID, Some(hir_id)); - } -} - -impl<'tcx> Visitor<'tcx> for LintLevelsBuilder<'_, QueryMapExpectationsWrapper<'tcx>> { - type NestedFilter = nested_filter::All; - - fn nested_visit_map(&mut self) -> Self::Map { - self.provider.tcx.hir() - } - - fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) { - self.add_id(param.hir_id); - intravisit::walk_param(self, param); - } - - fn visit_item(&mut self, it: &'tcx hir::Item<'tcx>) { - self.add_id(it.hir_id()); - intravisit::walk_item(self, it); - } - - fn visit_foreign_item(&mut self, it: &'tcx hir::ForeignItem<'tcx>) { - self.add_id(it.hir_id()); - intravisit::walk_foreign_item(self, it); - } - - fn visit_stmt(&mut self, e: &'tcx hir::Stmt<'tcx>) { - // We will call `add_id` when we walk - // the `StmtKind`. The outer statement itself doesn't - // define the lint levels. - intravisit::walk_stmt(self, e); - } - - fn visit_expr(&mut self, e: &'tcx hir::Expr<'tcx>) { - self.add_id(e.hir_id); - intravisit::walk_expr(self, e); - } - - fn visit_field_def(&mut self, s: &'tcx hir::FieldDef<'tcx>) { - self.add_id(s.hir_id); - intravisit::walk_field_def(self, s); - } - - fn visit_variant(&mut self, v: &'tcx hir::Variant<'tcx>) { - self.add_id(v.hir_id); - intravisit::walk_variant(self, v); - } - - fn visit_local(&mut self, l: &'tcx hir::LetStmt<'tcx>) { - self.add_id(l.hir_id); - intravisit::walk_local(self, l); - } - - fn visit_arm(&mut self, a: &'tcx hir::Arm<'tcx>) { - self.add_id(a.hir_id); - intravisit::walk_arm(self, a); - } - - fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem<'tcx>) { - self.add_id(trait_item.hir_id()); - intravisit::walk_trait_item(self, trait_item); - } - - fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem<'tcx>) { - self.add_id(impl_item.hir_id()); - intravisit::walk_impl_item(self, impl_item); - } -} - pub struct LintLevelsBuilder<'s, P> { sess: &'s Session, features: &'s Features, @@ -1099,7 +961,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { } pub(crate) fn provide(providers: &mut Providers) { - *providers = Providers { shallow_lint_levels_on, lint_expectations, ..*providers }; + *providers = Providers { shallow_lint_levels_on, ..*providers }; } pub(crate) fn parse_lint_and_tool_name(lint_name: &str) -> (Option, &str) { diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index 8c27cac1ea8..70da66af64b 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -6,7 +6,7 @@ use rustc_errors::{Diag, MultiSpan}; use rustc_hir::{HirId, ItemLocalId}; use rustc_macros::HashStable; use rustc_session::lint::builtin::{self, FORBIDDEN_LINT_GROUPS}; -use rustc_session::lint::{FutureIncompatibilityReason, Level, Lint, LintId}; +use rustc_session::lint::{FutureIncompatibilityReason, Level, Lint, LintExpectationId, LintId}; use rustc_session::Session; use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::{symbol, DesugaringKind, Span, Symbol, DUMMY_SP}; @@ -61,6 +61,7 @@ pub type LevelAndSource = (Level, LintLevelSource); /// by the attributes for *a single HirId*. #[derive(Default, Debug, HashStable)] pub struct ShallowLintLevelMap { + pub expectations: Vec<(LintExpectationId, LintExpectation)>, pub specs: SortedMap>, } diff --git a/tests/ui/error-codes/E0602.stderr b/tests/ui/error-codes/E0602.stderr index b0b6033aadd..b6b5cd5c3d3 100644 --- a/tests/ui/error-codes/E0602.stderr +++ b/tests/ui/error-codes/E0602.stderr @@ -13,11 +13,6 @@ warning[E0602]: unknown lint: `bogus` = note: requested on the command line with `-D bogus` = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -warning[E0602]: unknown lint: `bogus` - | - = note: requested on the command line with `-D bogus` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -warning: 4 warnings emitted +warning: 3 warnings emitted For more information about this error, try `rustc --explain E0602`. diff --git a/tests/ui/lint/cli-unknown-force-warn.stderr b/tests/ui/lint/cli-unknown-force-warn.stderr index cfff190b54a..5084b4a4001 100644 --- a/tests/ui/lint/cli-unknown-force-warn.stderr +++ b/tests/ui/lint/cli-unknown-force-warn.stderr @@ -13,11 +13,6 @@ warning[E0602]: unknown lint: `foo_qux` = note: requested on the command line with `--force-warn foo_qux` = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -warning[E0602]: unknown lint: `foo_qux` - | - = note: requested on the command line with `--force-warn foo_qux` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -warning: 4 warnings emitted +warning: 3 warnings emitted For more information about this error, try `rustc --explain E0602`. diff --git a/tests/ui/lint/lint-removed-cmdline-deny.stderr b/tests/ui/lint/lint-removed-cmdline-deny.stderr index 2a24e795f44..3321afa7fcd 100644 --- a/tests/ui/lint/lint-removed-cmdline-deny.stderr +++ b/tests/ui/lint/lint-removed-cmdline-deny.stderr @@ -26,10 +26,5 @@ LL | #[deny(warnings)] | ^^^^^^^^ = note: `#[deny(unused_variables)]` implied by `#[deny(warnings)]` -error: lint `raw_pointer_derive` has been removed: using derive with raw pointers is ok - | - = note: requested on the command line with `-D raw_pointer_derive` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error: aborting due to 5 previous errors +error: aborting due to 4 previous errors diff --git a/tests/ui/lint/lint-removed-cmdline.stderr b/tests/ui/lint/lint-removed-cmdline.stderr index 78ae2fd8fbf..fd63433c308 100644 --- a/tests/ui/lint/lint-removed-cmdline.stderr +++ b/tests/ui/lint/lint-removed-cmdline.stderr @@ -26,10 +26,5 @@ LL | #[deny(warnings)] | ^^^^^^^^ = note: `#[deny(unused_variables)]` implied by `#[deny(warnings)]` -warning: lint `raw_pointer_derive` has been removed: using derive with raw pointers is ok - | - = note: requested on the command line with `-D raw_pointer_derive` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error: aborting due to 1 previous error; 4 warnings emitted +error: aborting due to 1 previous error; 3 warnings emitted diff --git a/tests/ui/lint/lint-renamed-cmdline-deny.stderr b/tests/ui/lint/lint-renamed-cmdline-deny.stderr index 3c1a59ec1e1..0e182a4e5de 100644 --- a/tests/ui/lint/lint-renamed-cmdline-deny.stderr +++ b/tests/ui/lint/lint-renamed-cmdline-deny.stderr @@ -29,11 +29,5 @@ LL | #[deny(unused)] | ^^^^^^ = note: `#[deny(unused_variables)]` implied by `#[deny(unused)]` -error: lint `bare_trait_object` has been renamed to `bare_trait_objects` - | - = help: use the new name `bare_trait_objects` - = note: requested on the command line with `-D bare_trait_object` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error: aborting due to 5 previous errors +error: aborting due to 4 previous errors diff --git a/tests/ui/lint/lint-renamed-cmdline.stderr b/tests/ui/lint/lint-renamed-cmdline.stderr index 6544416f611..d6bb72f34dc 100644 --- a/tests/ui/lint/lint-renamed-cmdline.stderr +++ b/tests/ui/lint/lint-renamed-cmdline.stderr @@ -29,11 +29,5 @@ LL | #[deny(unused)] | ^^^^^^ = note: `#[deny(unused_variables)]` implied by `#[deny(unused)]` -warning: lint `bare_trait_object` has been renamed to `bare_trait_objects` - | - = help: use the new name `bare_trait_objects` - = note: requested on the command line with `-D bare_trait_object` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error: aborting due to 1 previous error; 4 warnings emitted +error: aborting due to 1 previous error; 3 warnings emitted diff --git a/tests/ui/lint/lint-unexported-no-mangle.stderr b/tests/ui/lint/lint-unexported-no-mangle.stderr index 39377b6fe84..0efec51abaf 100644 --- a/tests/ui/lint/lint-unexported-no-mangle.stderr +++ b/tests/ui/lint/lint-unexported-no-mangle.stderr @@ -45,15 +45,5 @@ LL | pub const PUB_FOO: u64 = 1; | | | help: try a static value: `pub static` -warning: lint `private_no_mangle_fns` has been removed: no longer a warning, `#[no_mangle]` functions always exported - | - = note: requested on the command line with `-F private_no_mangle_fns` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -warning: lint `private_no_mangle_statics` has been removed: no longer a warning, `#[no_mangle]` statics always exported - | - = note: requested on the command line with `-F private_no_mangle_statics` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error: aborting due to 2 previous errors; 8 warnings emitted +error: aborting due to 2 previous errors; 6 warnings emitted diff --git a/tests/ui/lint/lint-unknown-lint-cmdline-deny.stderr b/tests/ui/lint/lint-unknown-lint-cmdline-deny.stderr index 1ce55706d76..f12ce03ddfc 100644 --- a/tests/ui/lint/lint-unknown-lint-cmdline-deny.stderr +++ b/tests/ui/lint/lint-unknown-lint-cmdline-deny.stderr @@ -30,17 +30,6 @@ error[E0602]: unknown lint: `dead_cod` = note: requested on the command line with `-D dead_cod` = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -error[E0602]: unknown lint: `bogus` - | - = note: requested on the command line with `-D bogus` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error[E0602]: unknown lint: `dead_cod` - | - = help: did you mean: `dead_code` - = note: requested on the command line with `-D dead_cod` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error: aborting due to 8 previous errors +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0602`. diff --git a/tests/ui/lint/lint-unknown-lint-cmdline.stderr b/tests/ui/lint/lint-unknown-lint-cmdline.stderr index 4e0c5dbcb07..f452fc9eb94 100644 --- a/tests/ui/lint/lint-unknown-lint-cmdline.stderr +++ b/tests/ui/lint/lint-unknown-lint-cmdline.stderr @@ -30,17 +30,6 @@ warning[E0602]: unknown lint: `dead_cod` = note: requested on the command line with `-D dead_cod` = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -warning[E0602]: unknown lint: `bogus` - | - = note: requested on the command line with `-D bogus` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -warning[E0602]: unknown lint: `dead_cod` - | - = help: did you mean: `dead_code` - = note: requested on the command line with `-D dead_cod` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -warning: 8 warnings emitted +warning: 6 warnings emitted For more information about this error, try `rustc --explain E0602`. From 5f1f45b095f0980acd02bd3bbdc2a3ba9e8b8a38 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 4 Jul 2024 10:12:39 +0000 Subject: [PATCH 3/4] Remove attr_id from stable lint ids. --- compiler/rustc_errors/src/diagnostic.rs | 2 +- compiler/rustc_errors/src/lib.rs | 2 +- compiler/rustc_lint/src/expect.rs | 3 +-- compiler/rustc_lint/src/levels.rs | 18 +++++--------- compiler/rustc_lint_defs/src/lib.rs | 31 ++++--------------------- 5 files changed, 14 insertions(+), 42 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 7e4698a94a9..34aebadfdad 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -368,7 +368,7 @@ impl DiagInner { panic!("{expectation_id:?} must have a matching stable id") }; - let mut stable_id = stable_id.normalize(); + let mut stable_id = *stable_id; stable_id.set_lint_index(lint_index); *expectation_id = stable_id; } diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 967e64c2d9f..e08d28a156c 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1567,7 +1567,7 @@ impl DiagCtxtInner { if let LintExpectationId::Unstable { .. } = expect_id { unreachable!(); // this case was handled at the top of this function } - self.fulfilled_expectations.insert(expect_id.normalize()); + self.fulfilled_expectations.insert(expect_id); if let Expect(_) = diagnostic.level { // Nothing emitted here for expected lints. TRACK_DIAGNOSTIC(diagnostic, &mut |_| None); diff --git a/compiler/rustc_lint/src/expect.rs b/compiler/rustc_lint/src/expect.rs index d9fab1d00aa..d8afba3d505 100644 --- a/compiler/rustc_lint/src/expect.rs +++ b/compiler/rustc_lint/src/expect.rs @@ -20,8 +20,7 @@ fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExp 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, attr_id: None }; + 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| { diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index dfbc61d50e8..43682ad5a56 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -218,7 +218,7 @@ impl LintLevelsProvider for LintLevelQueryMap<'_> { self.specs.lint_level_id_at_node(self.tcx, LintId::of(lint), self.cur) } fn push_expectation(&mut self, id: LintExpectationId, expectation: LintExpectation) { - self.specs.expectations.push((id.normalize(), expectation)) + self.specs.expectations.push((id, expectation)) } } @@ -486,13 +486,9 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { /// Attempts to insert the `id` to `level_src` map entry. If unsuccessful /// (e.g. if a forbid was already inserted on the same scope), then emits a /// diagnostic with no change to `specs`. - fn insert_spec(&mut self, id: LintId, (mut level, src): LevelAndSource) { + fn insert_spec(&mut self, id: LintId, (level, src): LevelAndSource) { let (old_level, old_src) = self.provider.get_lint_level(id.lint, self.sess); - if let Level::Expect(id) = &mut level - && let LintExpectationId::Stable { .. } = id - { - *id = id.normalize(); - } + // Setting to a non-forbid level is an error if the lint previously had // a forbid level. Note that this is not necessarily true even with a // `#[forbid(..)]` attribute present, as that is overridden by `--cap-lints`. @@ -604,17 +600,15 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { // This is the only lint level with a `LintExpectationId` that can be created from // an attribute. Some(Level::Expect(unstable_id)) if let Some(hir_id) = source_hir_id => { - let LintExpectationId::Unstable { attr_id, lint_index } = unstable_id else { + let LintExpectationId::Unstable { lint_index: None, attr_id: _ } = unstable_id + else { bug!("stable id Level::from_attr") }; let stable_id = LintExpectationId::Stable { hir_id, attr_index: attr_index.try_into().unwrap(), - lint_index, - // We pass the previous unstable attr_id such that we can trace the ast id - // when building a map to go from unstable to stable id. - attr_id: Some(attr_id), + lint_index: None, }; Level::Expect(stable_id) diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index c17b85db3b0..d318f7aedb4 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -98,7 +98,7 @@ pub enum LintExpectationId { /// stable and can be cached. The additional index ensures that nodes with /// several expectations can correctly match diagnostics to the individual /// expectation. - Stable { hir_id: HirId, attr_index: u16, lint_index: Option, attr_id: Option }, + Stable { hir_id: HirId, attr_index: u16, lint_index: Option }, } impl LintExpectationId { @@ -122,31 +122,13 @@ impl LintExpectationId { *lint_index = new_lint_index } - - /// Prepares the id for hashing. Removes references to the ast. - /// Should only be called when the id is stable. - pub fn normalize(self) -> Self { - match self { - Self::Stable { hir_id, attr_index, lint_index, .. } => { - Self::Stable { hir_id, attr_index, lint_index, attr_id: None } - } - Self::Unstable { .. } => { - unreachable!("`normalize` called when `ExpectationId` is unstable") - } - } - } } impl HashStable for LintExpectationId { #[inline] fn hash_stable(&self, hcx: &mut HCX, hasher: &mut StableHasher) { match self { - LintExpectationId::Stable { - hir_id, - attr_index, - lint_index: Some(lint_index), - attr_id: _, - } => { + LintExpectationId::Stable { hir_id, attr_index, lint_index: Some(lint_index) } => { hir_id.hash_stable(hcx, hasher); attr_index.hash_stable(hcx, hasher); lint_index.hash_stable(hcx, hasher); @@ -166,12 +148,9 @@ impl ToStableHashKey for LintExpectation #[inline] fn to_stable_hash_key(&self, _: &HCX) -> Self::KeyType { match self { - LintExpectationId::Stable { - hir_id, - attr_index, - lint_index: Some(lint_index), - attr_id: _, - } => (*hir_id, *attr_index, *lint_index), + LintExpectationId::Stable { hir_id, attr_index, lint_index: Some(lint_index) } => { + (*hir_id, *attr_index, *lint_index) + } _ => { unreachable!("HashStable should only be called for a filled `LintExpectationId`") } From ff1fc68c11b04e7336c6deef210418f73efd6680 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 4 Jul 2024 10:33:42 +0000 Subject: [PATCH 4/4] Simplify lint source computation. --- compiler/rustc_lint/src/levels.rs | 188 +++++++++++++----------------- 1 file changed, 83 insertions(+), 105 deletions(-) diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index 43682ad5a56..65289de980e 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -695,59 +695,20 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { let name = pprust::path_to_string(&meta_item.path); let lint_result = self.store.check_lint_name(&name, tool_name, self.registered_tools); - match &lint_result { + + let (ids, name) = match lint_result { CheckLintNameResult::Ok(ids) => { - // This checks for instances where the user writes - // `#[expect(unfulfilled_lint_expectations)]` in that case we want to avoid - // overriding the lint level but instead add an expectation that can't be - // fulfilled. The lint message will include an explanation, that the - // `unfulfilled_lint_expectations` lint can't be expected. - if let Level::Expect(expect_id) = level { - // The `unfulfilled_lint_expectations` lint is not part of any lint - // groups. Therefore. we only need to check the slice if it contains a - // single lint. - let is_unfulfilled_lint_expectations = match ids { - [lint] => *lint == LintId::of(UNFULFILLED_LINT_EXPECTATIONS), - _ => false, - }; - self.provider.push_expectation( - expect_id, - LintExpectation::new( - reason, - sp, - is_unfulfilled_lint_expectations, - tool_name, - ), - ); - } - let src = LintLevelSource::Node { - name: meta_item - .path - .segments - .last() - .expect("empty lint name") - .ident - .name, - span: sp, - reason, - }; - for &id in *ids { - if self.check_gated_lint(id, attr.span, false) { - self.insert_spec(id, (level, src)); - } - } + let name = + meta_item.path.segments.last().expect("empty lint name").ident.name; + (ids, name) } CheckLintNameResult::Tool(ids, new_lint_name) => { - let src = match new_lint_name { + let name = match new_lint_name { None => { let complete_name = &format!("{}::{}", tool_ident.unwrap().name, name); - LintLevelSource::Node { - name: Symbol::intern(complete_name), - span: sp, - reason, - } + Symbol::intern(complete_name) } Some(new_lint_name) => { self.emit_span_lint( @@ -756,27 +717,13 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { DeprecatedLintName { name, suggestion: sp, - replace: new_lint_name, + replace: &new_lint_name, }, ); - LintLevelSource::Node { - name: Symbol::intern(new_lint_name), - span: sp, - reason, - } + Symbol::intern(&new_lint_name) } }; - for &id in *ids { - if self.check_gated_lint(id, attr.span, false) { - self.insert_spec(id, (level, src)); - } - } - if let Level::Expect(expect_id) = level { - self.provider.push_expectation( - expect_id, - LintExpectation::new(reason, sp, false, tool_name), - ); - } + (ids, name) } CheckLintNameResult::MissingTool => { @@ -784,9 +731,10 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { // exist in the tool or the code was not compiled with the tool and // therefore the lint was never added to the `LintStore`. To detect // this is the responsibility of the lint tool. + continue; } - &CheckLintNameResult::NoTool => { + CheckLintNameResult::NoTool => { sess.dcx().emit_err(UnknownToolInScopedLint { span: tool_ident.map(|ident| ident.span), tool_name: tool_name.unwrap(), @@ -796,57 +744,87 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { continue; } - _ if !self.lint_added_lints => {} - CheckLintNameResult::Renamed(ref replace) => { - let suggestion = - RenamedLintSuggestion::WithSpan { suggestion: sp, replace }; - let name = tool_ident.map(|tool| format!("{tool}::{name}")).unwrap_or(name); - let lint = RenamedLint { name: name.as_str(), suggestion }; - self.emit_span_lint(RENAMED_AND_REMOVED_LINTS, sp.into(), lint); + if self.lint_added_lints { + let suggestion = + RenamedLintSuggestion::WithSpan { suggestion: sp, replace }; + let name = + tool_ident.map(|tool| format!("{tool}::{name}")).unwrap_or(name); + let lint = RenamedLint { name: name.as_str(), suggestion }; + self.emit_span_lint(RENAMED_AND_REMOVED_LINTS, sp.into(), lint); + } + + // If this lint was renamed, apply the new lint instead of ignoring the + // attribute. Ignore any errors or warnings that happen because the new + // name is inaccurate. + // NOTE: `new_name` already includes the tool name, so we don't + // have to add it again. + let CheckLintNameResult::Ok(ids) = + self.store.check_lint_name(replace, None, self.registered_tools) + else { + panic!("renamed lint does not exist: {replace}"); + }; + + (ids, Symbol::intern(&replace)) } CheckLintNameResult::Removed(ref reason) => { - let name = tool_ident.map(|tool| format!("{tool}::{name}")).unwrap_or(name); - let lint = RemovedLint { name: name.as_str(), reason }; - self.emit_span_lint(RENAMED_AND_REMOVED_LINTS, sp.into(), lint); + if self.lint_added_lints { + let name = + tool_ident.map(|tool| format!("{tool}::{name}")).unwrap_or(name); + let lint = RemovedLint { name: name.as_str(), reason }; + self.emit_span_lint(RENAMED_AND_REMOVED_LINTS, sp.into(), lint); + } + continue; } CheckLintNameResult::NoLint(suggestion) => { - let name = tool_ident.map(|tool| format!("{tool}::{name}")).unwrap_or(name); - let suggestion = suggestion.map(|(replace, from_rustc)| { - UnknownLintSuggestion::WithSpan { suggestion: sp, replace, from_rustc } - }); - let lint = UnknownLint { name, suggestion }; - self.emit_span_lint(UNKNOWN_LINTS, sp.into(), lint); + if self.lint_added_lints { + let name = + tool_ident.map(|tool| format!("{tool}::{name}")).unwrap_or(name); + let suggestion = suggestion.map(|(replace, from_rustc)| { + UnknownLintSuggestion::WithSpan { + suggestion: sp, + replace, + from_rustc, + } + }); + let lint = UnknownLint { name, suggestion }; + self.emit_span_lint(UNKNOWN_LINTS, sp.into(), lint); + } + continue; + } + }; + + let src = LintLevelSource::Node { name, span: sp, reason }; + for &id in ids { + if self.check_gated_lint(id, attr.span, false) { + self.insert_spec(id, (level, src)); } } - // If this lint was renamed, apply the new lint instead of ignoring the attribute. - // This happens outside of the match because the new lint should be applied even if - // we don't warn about the name change. - if let CheckLintNameResult::Renamed(new_name) = lint_result { - // Ignore any errors or warnings that happen because the new name is inaccurate - // NOTE: `new_name` already includes the tool name, so we don't have to add it - // again. - let CheckLintNameResult::Ok(ids) = - self.store.check_lint_name(&new_name, None, self.registered_tools) - else { - panic!("renamed lint does not exist: {new_name}"); - }; - let src = - LintLevelSource::Node { name: Symbol::intern(&new_name), span: sp, reason }; - for &id in ids { - if self.check_gated_lint(id, attr.span, false) { - self.insert_spec(id, (level, src)); - } - } - if let Level::Expect(expect_id) = level { - self.provider.push_expectation( - expect_id, - LintExpectation::new(reason, sp, false, tool_name), - ); - } + // This checks for instances where the user writes + // `#[expect(unfulfilled_lint_expectations)]` in that case we want to avoid + // overriding the lint level but instead add an expectation that can't be + // fulfilled. The lint message will include an explanation, that the + // `unfulfilled_lint_expectations` lint can't be expected. + if let Level::Expect(expect_id) = level { + // The `unfulfilled_lint_expectations` lint is not part of any lint + // groups. Therefore. we only need to check the slice if it contains a + // single lint. + let is_unfulfilled_lint_expectations = match ids { + [lint] => *lint == LintId::of(UNFULFILLED_LINT_EXPECTATIONS), + _ => false, + }; + self.provider.push_expectation( + expect_id, + LintExpectation::new( + reason, + sp, + is_unfulfilled_lint_expectations, + tool_name, + ), + ); } } }