From eb19a8a6208d992d6d4e661be8fa80c1eeab01f6 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Fri, 22 Jul 2022 16:48:36 +0000 Subject: [PATCH 1/8] Compute `lint_levels` by definition --- compiler/rustc_errors/src/diagnostic.rs | 5 +- compiler/rustc_errors/src/lib.rs | 2 +- compiler/rustc_lint/src/context.rs | 2 +- compiler/rustc_lint/src/early.rs | 1 + compiler/rustc_lint/src/expect.rs | 4 +- compiler/rustc_lint/src/levels.rs | 588 +++++++++--------- compiler/rustc_lint_defs/src/lib.rs | 31 +- compiler/rustc_middle/src/lint.rs | 111 +++- compiler/rustc_middle/src/query/mod.rs | 10 +- compiler/rustc_middle/src/ty/context.rs | 23 +- compiler/rustc_middle/src/ty/query.rs | 4 +- compiler/rustc_query_impl/src/keys.rs | 17 + ...force_warn_expected_lints_fulfilled.stderr | 16 +- 13 files changed, 482 insertions(+), 332 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index a774b52c8a592..6dfb6b11b1646 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -338,9 +338,10 @@ impl Diagnostic { // 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 + let mut stable_id = unstable_to_stable .get(&expectation_id) - .expect("each unstable `LintExpectationId` must have a matching stable id"); + .expect("each unstable `LintExpectationId` must have a matching 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 888128f3f74e7..1b24f07e689bb 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1167,7 +1167,7 @@ impl HandlerInner { if let Some(expectation_id) = diagnostic.level.get_expectation_id() { self.suppressed_expected_diag = true; - self.fulfilled_expectations.insert(expectation_id); + self.fulfilled_expectations.insert(expectation_id.normalize()); } if matches!(diagnostic.level, Warning(_)) diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index 7ca6ec5d96234..34506c84cb8c7 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -560,7 +560,7 @@ pub struct LateContext<'tcx> { /// Context for lint checking of the AST, after expansion, before lowering to HIR. pub struct EarlyContext<'a> { - pub builder: LintLevelsBuilder<'a>, + pub builder: LintLevelsBuilder<'a, crate::levels::TopDown>, pub buffered: LintBuffer, } diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index 96ecd79a69cfb..45f54cfed2d59 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -59,6 +59,7 @@ impl<'a, T: EarlyLintPass> EarlyContextAndPass<'a, T> { F: FnOnce(&mut Self), { let is_crate_node = id == ast::CRATE_NODE_ID; + debug!(?id); let push = self.context.builder.push(attrs, is_crate_node, None); self.check_id(id); diff --git a/compiler/rustc_lint/src/expect.rs b/compiler/rustc_lint/src/expect.rs index 699e81543188f..b9a3d52ca9b19 100644 --- a/compiler/rustc_lint/src/expect.rs +++ b/compiler/rustc_lint/src/expect.rs @@ -16,8 +16,10 @@ fn check_expectations(tcx: TyCtxt<'_>, tool_filter: Option) { return; } + let lint_expectations = tcx.lint_expectations(()); let fulfilled_expectations = tcx.sess.diagnostic().steal_fulfilled_expectation_ids(); - let lint_expectations = &tcx.lint_levels(()).lint_expectations; + + tracing::debug!(?lint_expectations, ?fulfilled_expectations); for (id, expectation) in lint_expectations { // This check will always be true, since `lint_expectations` only diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index 1e16ac51e9e5d..1626a4a99df79 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -8,7 +8,7 @@ use rustc_hir as hir; use rustc_hir::{intravisit, HirId}; use rustc_middle::hir::nested_filter; use rustc_middle::lint::{ - struct_lint_level, LevelAndSource, LintExpectation, LintLevelMap, LintLevelSets, + struct_lint_level, LevelAndSource, LintExpectation, LintLevelQueryMap, LintLevelSets, LintLevelSource, LintSet, LintStackIndex, COMMAND_LINE, }; use rustc_middle::ty::query::Providers; @@ -27,35 +27,199 @@ use crate::errors::{ UnknownToolInScopedLint, }; -fn lint_levels(tcx: TyCtxt<'_>, (): ()) -> LintLevelMap { +fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExpectation)> { let store = unerased_lint_store(tcx); - let levels = - LintLevelsBuilder::new(tcx.sess, false, &store, &tcx.resolutions(()).registered_tools); - let mut builder = LintLevelMapBuilder { levels, tcx }; - let krate = tcx.hir().krate(); - builder.levels.id_to_set.reserve(krate.owners.len() + 1); + let mut builder = LintLevelsBuilder { + sess: tcx.sess, + provider: QueryMapExpectationsWrapper { + map: LintLevelQueryMap { tcx, cur: hir::CRATE_HIR_ID, specs: FxHashMap::default() }, + expectations: Vec::new(), + unstable_to_stable_ids: FxHashMap::default(), + }, + warn_about_weird_lints: false, + store, + registered_tools: &tcx.resolutions(()).registered_tools, + }; + + builder.add_command_line(); + builder.add_id(hir::CRATE_HIR_ID); + tcx.hir().walk_toplevel_module(&mut builder); - let push = - builder.levels.push(tcx.hir().attrs(hir::CRATE_HIR_ID), true, Some(hir::CRATE_HIR_ID)); + tcx.sess.diagnostic().update_unstable_expectation_id(&builder.provider.unstable_to_stable_ids); - builder.levels.register_id(hir::CRATE_HIR_ID); - tcx.hir().walk_toplevel_module(&mut builder); - builder.levels.pop(push); + builder.provider.expectations +} + +fn lint_levels_on(tcx: TyCtxt<'_>, hir_id: HirId) -> FxHashMap { + let store = unerased_lint_store(tcx); + + let mut levels = LintLevelsBuilder { + sess: tcx.sess, + provider: LintLevelQueryMap { tcx, cur: hir_id, specs: FxHashMap::default() }, + warn_about_weird_lints: false, + store, + registered_tools: &tcx.resolutions(()).registered_tools, + }; + + let is_crate = hir::CRATE_HIR_ID == hir_id; + if is_crate { + levels.add_command_line(); + } + debug!(?hir_id); + levels.add(tcx.hir().attrs(hir_id), is_crate, Some(hir_id)); - builder.levels.update_unstable_expectation_ids(); - builder.levels.build_map() + levels.provider.specs } -pub struct LintLevelsBuilder<'s> { - sess: &'s Session, - lint_expectations: Vec<(LintExpectationId, LintExpectation)>, - /// Each expectation has a stable and an unstable identifier. This map - /// is used to map from unstable to stable [`LintExpectationId`]s. - expectation_id_map: FxHashMap, +pub struct TopDown { sets: LintLevelSets, - id_to_set: FxHashMap, cur: LintStackIndex, +} + +pub struct QueryMapExpectationsWrapper<'tcx> { + map: LintLevelQueryMap<'tcx>, + expectations: Vec<(LintExpectationId, LintExpectation)>, + unstable_to_stable_ids: FxHashMap, +} + +pub trait LintLevelsProvider { + fn current_specs(&self) -> &FxHashMap; + fn current_specs_mut(&mut self) -> &mut FxHashMap; + fn get_lint_level(&self, lint: &'static Lint, sess: &Session) -> LevelAndSource; + fn push_expectation(&mut self, _id: LintExpectationId, _expectation: LintExpectation) {} +} + +impl LintLevelsProvider for TopDown { + fn current_specs(&self) -> &FxHashMap { + &self.sets.list[self.cur].specs + } + + fn current_specs_mut(&mut self) -> &mut FxHashMap { + &mut self.sets.list[self.cur].specs + } + + fn get_lint_level(&self, lint: &'static Lint, sess: &Session) -> LevelAndSource { + self.sets.get_lint_level(lint, self.cur, Some(self.current_specs()), sess) + } +} + +impl LintLevelsProvider for LintLevelQueryMap<'_> { + fn current_specs(&self) -> &FxHashMap { + &self.specs + } + fn current_specs_mut(&mut self) -> &mut FxHashMap { + &mut self.specs + } + fn get_lint_level(&self, lint: &'static Lint, _: &Session) -> LevelAndSource { + self.lint_level(lint) + } +} + +impl LintLevelsProvider for QueryMapExpectationsWrapper<'_> { + fn current_specs(&self) -> &FxHashMap { + &self.map.specs + } + fn current_specs_mut(&mut self) -> &mut FxHashMap { + self.map.specs.clear(); + &mut self.map.specs + } + fn get_lint_level(&self, lint: &'static Lint, _: &Session) -> LevelAndSource { + self.map.lint_level(lint) + } + 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") }; + let key = LintExpectationId::Unstable { attr_id, lint_index: None }; + + if !self.unstable_to_stable_ids.contains_key(&key) { + self.unstable_to_stable_ids.insert( + key, + LintExpectationId::Stable { hir_id, attr_index, lint_index: None, attr_id: None }, + ); + } + + self.expectations.push((id.normalize(), expectation)); + } +} + +impl<'tcx> LintLevelsBuilder<'_, QueryMapExpectationsWrapper<'tcx>> { + fn add_id(&mut self, hir_id: HirId) { + self.add( + self.provider.map.tcx.hir().attrs(hir_id), + hir_id == hir::CRATE_HIR_ID, + Some(hir_id), + ); + } +} + +impl<'tcx> intravisit::Visitor<'tcx> for LintLevelsBuilder<'_, QueryMapExpectationsWrapper<'tcx>> { + type NestedFilter = nested_filter::All; + + fn nested_visit_map(&mut self) -> Self::Map { + self.provider.map.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.id); + intravisit::walk_variant(self, v); + } + + fn visit_local(&mut self, l: &'tcx hir::Local<'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, + provider: P, warn_about_weird_lints: bool, store: &'s LintStore, registered_tools: &'s RegisteredTools, @@ -66,7 +230,7 @@ pub struct BuilderPush { pub changed: bool, } -impl<'s> LintLevelsBuilder<'s> { +impl<'s> LintLevelsBuilder<'s, TopDown> { pub fn new( sess: &'s Session, warn_about_weird_lints: bool, @@ -75,20 +239,66 @@ impl<'s> LintLevelsBuilder<'s> { ) -> Self { let mut builder = LintLevelsBuilder { sess, - lint_expectations: Default::default(), - expectation_id_map: Default::default(), - sets: LintLevelSets::new(), - cur: COMMAND_LINE, - id_to_set: Default::default(), + provider: TopDown { sets: LintLevelSets::new(), cur: COMMAND_LINE }, warn_about_weird_lints, store, registered_tools, }; - builder.process_command_line(sess, store); - assert_eq!(builder.sets.list.len(), 1); + builder.process_command_line(); + assert_eq!(builder.provider.sets.list.len(), 1); builder } + fn process_command_line(&mut self) { + self.provider.cur = self + .provider + .sets + .list + .push(LintSet { specs: FxHashMap::default(), parent: COMMAND_LINE }); + self.add_command_line(); + } + + /// Pushes a list of AST lint attributes onto this context. + /// + /// This function will return a `BuilderPush` object which should be passed + /// to `pop` when this scope for the attributes provided is exited. + /// + /// This function will perform a number of tasks: + /// + /// * It'll validate all lint-related attributes in `attrs` + /// * It'll mark all lint-related attributes as used + /// * Lint levels will be updated based on the attributes provided + /// * Lint attributes are validated, e.g., a `#[forbid]` can't be switched to + /// `#[allow]` + /// + /// Don't forget to call `pop`! + pub(crate) fn push( + &mut self, + attrs: &[ast::Attribute], + is_crate_node: bool, + source_hir_id: Option, + ) -> BuilderPush { + let prev = self.provider.cur; + self.provider.cur = + self.provider.sets.list.push(LintSet { specs: FxHashMap::default(), parent: prev }); + + self.add(attrs, is_crate_node, source_hir_id); + + if self.provider.current_specs().is_empty() { + self.provider.sets.list.pop(); + self.provider.cur = prev; + } + + BuilderPush { prev, changed: prev != self.provider.cur } + } + + /// Called after `push` when the scope of a set of attributes are exited. + pub fn pop(&mut self, push: BuilderPush) { + self.provider.cur = push.prev; + } +} + +impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { pub(crate) fn sess(&self) -> &Session { self.sess } @@ -98,24 +308,20 @@ impl<'s> LintLevelsBuilder<'s> { } fn current_specs(&self) -> &FxHashMap { - &self.sets.list[self.cur].specs + self.provider.current_specs() } fn current_specs_mut(&mut self) -> &mut FxHashMap { - &mut self.sets.list[self.cur].specs + self.provider.current_specs_mut() } - fn process_command_line(&mut self, sess: &Session, store: &LintStore) { - self.sets.lint_cap = sess.opts.lint_cap.unwrap_or(Level::Forbid); - - self.cur = - self.sets.list.push(LintSet { specs: FxHashMap::default(), parent: COMMAND_LINE }); - for &(ref lint_name, level) in &sess.opts.lint_opts { - store.check_lint_name_cmdline(sess, &lint_name, level, self.registered_tools); + fn add_command_line(&mut self) { + for &(ref lint_name, level) in &self.sess.opts.lint_opts { + self.store.check_lint_name_cmdline(self.sess, &lint_name, level, self.registered_tools); let orig_level = level; let lint_flag_val = Symbol::intern(lint_name); - let Ok(ids) = store.find_lints(&lint_name) else { + let Ok(ids) = self.store.find_lints(&lint_name) else { // errors handled in check_lint_name_cmdline above continue }; @@ -138,9 +344,11 @@ impl<'s> LintLevelsBuilder<'s> { /// 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, (level, src): LevelAndSource) { - let (old_level, old_src) = - self.sets.get_lint_level(id.lint, self.cur, Some(self.current_specs()), &self.sess); + fn insert_spec(&mut self, id: LintId, (mut 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`. @@ -158,7 +366,7 @@ impl<'s> LintLevelsBuilder<'s> { let id_name = id.lint.name_lower(); let fcw_warning = match old_src { LintLevelSource::Default => false, - LintLevelSource::Node(symbol, _, _) => self.store.is_lint_group(symbol), + LintLevelSource::Node { name, .. } => self.store.is_lint_group(name), LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol), }; debug!( @@ -178,8 +386,8 @@ impl<'s> LintLevelsBuilder<'s> { id.to_string() )); } - LintLevelSource::Node(_, forbid_source_span, reason) => { - diag.span_label(forbid_source_span, "`forbid` level set here"); + LintLevelSource::Node { span, reason, .. } => { + diag.span_label(span, "`forbid` level set here"); if let Some(rationale) = reason { diag.note(rationale.as_str()); } @@ -199,11 +407,8 @@ impl<'s> LintLevelsBuilder<'s> { LintLevelSource::Default => { OverruledAttributeSub::DefaultSource { id: id.to_string() } } - LintLevelSource::Node(_, forbid_source_span, reason) => { - OverruledAttributeSub::NodeSource { - span: forbid_source_span, - reason, - } + LintLevelSource::Node { span, reason, .. } => { + OverruledAttributeSub::NodeSource { span, reason } } LintLevelSource::CommandLine(_, _) => { OverruledAttributeSub::CommandLineSource @@ -256,29 +461,7 @@ impl<'s> LintLevelsBuilder<'s> { }; } - /// Pushes a list of AST lint attributes onto this context. - /// - /// This function will return a `BuilderPush` object which should be passed - /// to `pop` when this scope for the attributes provided is exited. - /// - /// This function will perform a number of tasks: - /// - /// * It'll validate all lint-related attributes in `attrs` - /// * It'll mark all lint-related attributes as used - /// * Lint levels will be updated based on the attributes provided - /// * Lint attributes are validated, e.g., a `#[forbid]` can't be switched to - /// `#[allow]` - /// - /// Don't forget to call `pop`! - pub(crate) fn push( - &mut self, - attrs: &[ast::Attribute], - is_crate_node: bool, - source_hir_id: Option, - ) -> BuilderPush { - let prev = self.cur; - self.cur = self.sets.list.push(LintSet { specs: FxHashMap::default(), parent: prev }); - + fn add(&mut self, attrs: &[ast::Attribute], is_crate_node: bool, source_hir_id: Option) { let sess = self.sess; for (attr_index, attr) in attrs.iter().enumerate() { if attr.has_name(sym::automatically_derived) { @@ -293,7 +476,17 @@ impl<'s> LintLevelsBuilder<'s> { None => continue, // 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 stable_id = self.create_stable_id(unstable_id, hir_id, attr_index); + let LintExpectationId::Unstable { attr_id, lint_index } = 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), + }; Level::Expect(stable_id) } @@ -408,7 +601,7 @@ impl<'s> LintLevelsBuilder<'s> { [lint] => *lint == LintId::of(UNFULFILLED_LINT_EXPECTATIONS), _ => false, }; - self.lint_expectations.push(( + self.provider.push_expectation( expect_id, LintExpectation::new( reason, @@ -416,13 +609,20 @@ impl<'s> LintLevelsBuilder<'s> { is_unfulfilled_lint_expectations, tool_name, ), - )); + ); } - let src = LintLevelSource::Node( - meta_item.path.segments.last().expect("empty lint name").ident.name, - sp, + let src = LintLevelSource::Node { + name: meta_item + .path + .segments + .last() + .expect("empty lint name") + .ident + .name, + span: sp, reason, - ); + tool: tool_name, + }; for &id in *ids { if self.check_gated_lint(id, attr.span) { self.insert_spec(id, (level, src)); @@ -435,31 +635,27 @@ impl<'s> LintLevelsBuilder<'s> { Ok(ids) => { let complete_name = &format!("{}::{}", tool_ident.unwrap().name, name); - let src = LintLevelSource::Node( - Symbol::intern(complete_name), - sp, + let src = LintLevelSource::Node { + name: Symbol::intern(complete_name), + span: sp, reason, - ); + tool: tool_name, + }; for &id in ids { if self.check_gated_lint(id, attr.span) { self.insert_spec(id, (level, src)); } } if let Level::Expect(expect_id) = level { - self.lint_expectations.push(( + self.provider.push_expectation( expect_id, LintExpectation::new(reason, sp, false, tool_name), - )); + ); } } Err((Some(ids), ref new_lint_name)) => { let lint = builtin::RENAMED_AND_REMOVED_LINTS; - let (lvl, src) = self.sets.get_lint_level( - lint, - self.cur, - Some(self.current_specs()), - &sess, - ); + let (lvl, src) = self.provider.get_lint_level(lint, &sess); struct_lint_level( self.sess, lint, @@ -483,19 +679,20 @@ impl<'s> LintLevelsBuilder<'s> { }, ); - let src = LintLevelSource::Node( - Symbol::intern(&new_lint_name), - sp, + let src = LintLevelSource::Node { + name: Symbol::intern(&new_lint_name), + span: sp, reason, - ); + tool: tool_name, + }; for id in ids { self.insert_spec(*id, (level, src)); } if let Level::Expect(expect_id) = level { - self.lint_expectations.push(( + self.provider.push_expectation( expect_id, LintExpectation::new(reason, sp, false, tool_name), - )); + ); } } Err((None, _)) => { @@ -521,12 +718,7 @@ impl<'s> LintLevelsBuilder<'s> { CheckLintNameResult::Warning(msg, renamed) => { let lint = builtin::RENAMED_AND_REMOVED_LINTS; - let (renamed_lint_level, src) = self.sets.get_lint_level( - lint, - self.cur, - Some(self.current_specs()), - &sess, - ); + let (renamed_lint_level, src) = self.provider.get_lint_level(lint, &sess); struct_lint_level( self.sess, lint, @@ -549,12 +741,7 @@ impl<'s> LintLevelsBuilder<'s> { } CheckLintNameResult::NoLint(suggestion) => { let lint = builtin::UNKNOWN_LINTS; - let (level, src) = self.sets.get_lint_level( - lint, - self.cur, - Some(self.current_specs()), - self.sess, - ); + let (level, src) = self.provider.get_lint_level(lint, self.sess); struct_lint_level(self.sess, lint, level, src, Some(sp.into()), |lint| { let name = if let Some(tool_ident) = tool_ident { format!("{}::{}", tool_ident.name, name) @@ -583,17 +770,22 @@ impl<'s> LintLevelsBuilder<'s> { if let CheckLintNameResult::Ok(ids) = self.store.check_lint_name(&new_name, None, self.registered_tools) { - let src = LintLevelSource::Node(Symbol::intern(&new_name), sp, reason); + let src = LintLevelSource::Node { + name: Symbol::intern(&new_name), + span: sp, + reason, + tool: tool_name, + }; for &id in ids { if self.check_gated_lint(id, attr.span) { self.insert_spec(id, (level, src)); } } if let Level::Expect(expect_id) = level { - self.lint_expectations.push(( + self.provider.push_expectation( expect_id, LintExpectation::new(reason, sp, false, tool_name), - )); + ); } } else { panic!("renamed lint does not exist: {}", new_name); @@ -608,13 +800,12 @@ impl<'s> LintLevelsBuilder<'s> { continue; } - let LintLevelSource::Node(lint_attr_name, lint_attr_span, _) = *src else { + let LintLevelSource::Node { name: lint_attr_name, span: lint_attr_span, .. } = *src else { continue }; let lint = builtin::UNUSED_ATTRIBUTES; - let (lint_level, lint_src) = - self.sets.get_lint_level(lint, self.cur, Some(self.current_specs()), self.sess); + let (lint_level, lint_src) = self.provider.get_lint_level(lint, &self.sess); struct_lint_level( self.sess, lint, @@ -634,27 +825,6 @@ impl<'s> LintLevelsBuilder<'s> { break; } } - - if self.current_specs().is_empty() { - self.sets.list.pop(); - self.cur = prev; - } - - BuilderPush { prev, changed: prev != self.cur } - } - - fn create_stable_id( - &mut self, - unstable_id: LintExpectationId, - hir_id: HirId, - attr_index: usize, - ) -> LintExpectationId { - let stable_id = - LintExpectationId::Stable { hir_id, attr_index: attr_index as u16, lint_index: None }; - - self.expectation_id_map.insert(unstable_id, stable_id); - - stable_id } /// Checks if the lint is gated on a feature that is not enabled. @@ -678,14 +848,9 @@ impl<'s> LintLevelsBuilder<'s> { true } - /// Called after `push` when the scope of a set of attributes are exited. - pub fn pop(&mut self, push: BuilderPush) { - self.cur = push.prev; - } - /// Find the lint level for a lint. - pub fn lint_level(&self, lint: &'static Lint) -> (Level, LintLevelSource) { - self.sets.get_lint_level(lint, self.cur, None, self.sess) + pub fn lint_level(&self, lint: &'static Lint) -> LevelAndSource { + self.provider.get_lint_level(lint, self.sess) } /// Used to emit a lint-related diagnostic based on the current state of @@ -699,141 +864,8 @@ impl<'s> LintLevelsBuilder<'s> { let (level, src) = self.lint_level(lint); struct_lint_level(self.sess, lint, level, src, span, decorate) } - - /// Registers the ID provided with the current set of lints stored in - /// this context. - pub fn register_id(&mut self, id: HirId) { - self.id_to_set.insert(id, self.cur); - } - - fn update_unstable_expectation_ids(&self) { - self.sess.diagnostic().update_unstable_expectation_id(&self.expectation_id_map); - } - - pub fn build_map(self) -> LintLevelMap { - LintLevelMap { - sets: self.sets, - id_to_set: self.id_to_set, - lint_expectations: self.lint_expectations, - } - } -} - -struct LintLevelMapBuilder<'tcx> { - levels: LintLevelsBuilder<'tcx>, - tcx: TyCtxt<'tcx>, -} - -impl LintLevelMapBuilder<'_> { - fn with_lint_attrs(&mut self, id: hir::HirId, f: F) - where - F: FnOnce(&mut Self), - { - let is_crate_hir = id == hir::CRATE_HIR_ID; - let attrs = self.tcx.hir().attrs(id); - let push = self.levels.push(attrs, is_crate_hir, Some(id)); - - if push.changed { - self.levels.register_id(id); - } - f(self); - self.levels.pop(push); - } -} - -impl<'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'tcx> { - type NestedFilter = nested_filter::All; - - fn nested_visit_map(&mut self) -> Self::Map { - self.tcx.hir() - } - - fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) { - self.with_lint_attrs(param.hir_id, |builder| { - intravisit::walk_param(builder, param); - }); - } - - fn visit_item(&mut self, it: &'tcx hir::Item<'tcx>) { - self.with_lint_attrs(it.hir_id(), |builder| { - intravisit::walk_item(builder, it); - }); - } - - fn visit_foreign_item(&mut self, it: &'tcx hir::ForeignItem<'tcx>) { - self.with_lint_attrs(it.hir_id(), |builder| { - intravisit::walk_foreign_item(builder, it); - }) - } - - fn visit_stmt(&mut self, e: &'tcx hir::Stmt<'tcx>) { - // We will call `with_lint_attrs` 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.with_lint_attrs(e.hir_id, |builder| { - intravisit::walk_expr(builder, e); - }) - } - - fn visit_expr_field(&mut self, field: &'tcx hir::ExprField<'tcx>) { - self.with_lint_attrs(field.hir_id, |builder| { - intravisit::walk_expr_field(builder, field); - }) - } - - fn visit_field_def(&mut self, s: &'tcx hir::FieldDef<'tcx>) { - self.with_lint_attrs(s.hir_id, |builder| { - intravisit::walk_field_def(builder, s); - }) - } - - fn visit_variant(&mut self, v: &'tcx hir::Variant<'tcx>) { - self.with_lint_attrs(v.id, |builder| { - intravisit::walk_variant(builder, v); - }) - } - - fn visit_local(&mut self, l: &'tcx hir::Local<'tcx>) { - self.with_lint_attrs(l.hir_id, |builder| { - intravisit::walk_local(builder, l); - }) - } - - fn visit_arm(&mut self, a: &'tcx hir::Arm<'tcx>) { - self.with_lint_attrs(a.hir_id, |builder| { - intravisit::walk_arm(builder, a); - }) - } - - fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem<'tcx>) { - self.with_lint_attrs(trait_item.hir_id(), |builder| { - intravisit::walk_trait_item(builder, trait_item); - }); - } - - fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem<'tcx>) { - self.with_lint_attrs(impl_item.hir_id(), |builder| { - intravisit::walk_impl_item(builder, impl_item); - }); - } - - fn visit_pat_field(&mut self, field: &'tcx hir::PatField<'tcx>) { - self.with_lint_attrs(field.hir_id, |builder| { - intravisit::walk_pat_field(builder, field); - }) - } - - fn visit_generic_param(&mut self, p: &'tcx hir::GenericParam<'tcx>) { - self.with_lint_attrs(p.hir_id, |builder| { - intravisit::walk_generic_param(builder, p); - }); - } } pub fn provide(providers: &mut Providers) { - providers.lint_levels = lint_levels; + *providers = Providers { lint_levels_on, lint_expectations, ..*providers }; } diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 11b2d057a0769..cbe7afc8e5558 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -92,7 +92,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 }, + Stable { hir_id: HirId, attr_index: u16, lint_index: Option, attr_id: Option }, } impl LintExpectationId { @@ -116,13 +116,31 @@ 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) } => { + LintExpectationId::Stable { + hir_id, + attr_index, + lint_index: Some(lint_index), + attr_id: _, + } => { hir_id.hash_stable(hcx, hasher); attr_index.hash_stable(hcx, hasher); lint_index.hash_stable(hcx, hasher); @@ -142,9 +160,12 @@ 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) } => { - (*hir_id, *attr_index, *lint_index) - } + LintExpectationId::Stable { + hir_id, + attr_index, + lint_index: Some(lint_index), + attr_id: _, + } => (*hir_id, *attr_index, *lint_index), _ => { unreachable!("HashStable should only be called for a filled `LintExpectationId`") } diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index 2f45222de4728..c659e677fa0e1 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -15,6 +15,8 @@ use rustc_span::hygiene::MacroKind; use rustc_span::source_map::{DesugaringKind, ExpnKind}; use rustc_span::{symbol, Span, Symbol, DUMMY_SP}; +use crate::ty::TyCtxt; + /// How a lint level was set. #[derive(Clone, Copy, PartialEq, Eq, HashStable, Debug)] pub enum LintLevelSource { @@ -23,7 +25,14 @@ pub enum LintLevelSource { Default, /// Lint level was set by an attribute. - Node(Symbol, Span, Option /* RFC 2383 reason */), + Node { + name: Symbol, + span: Span, + /// RFC 2383 reason + reason: Option, + /// The lint tool. (e.g. rustdoc, clippy) + tool: Option, + }, /// Lint level was set by a command-line flag. /// The provided `Level` is the level specified on the command line. @@ -35,7 +44,7 @@ impl LintLevelSource { pub fn name(&self) -> Symbol { match *self { LintLevelSource::Default => symbol::kw::Default, - LintLevelSource::Node(name, _, _) => name, + LintLevelSource::Node { name, .. } => name, LintLevelSource::CommandLine(name, _) => name, } } @@ -43,7 +52,7 @@ impl LintLevelSource { pub fn span(&self) -> Span { match *self { LintLevelSource::Default => DUMMY_SP, - LintLevelSource::Node(_, span, _) => span, + LintLevelSource::Node { span, .. } => span, LintLevelSource::CommandLine(_, _) => DUMMY_SP, } } @@ -55,7 +64,6 @@ pub type LevelAndSource = (Level, LintLevelSource); #[derive(Debug, HashStable)] pub struct LintLevelSets { pub list: IndexVec, - pub lint_cap: Level, } rustc_index::newtype_index! { @@ -76,18 +84,16 @@ pub struct LintSet { impl LintLevelSets { pub fn new() -> Self { - LintLevelSets { list: IndexVec::new(), lint_cap: Level::Forbid } + LintLevelSets { list: IndexVec::new() } } - pub fn get_lint_level( - &self, - lint: &'static Lint, - idx: LintStackIndex, - aux: Option<&FxHashMap>, + pub fn actual_level( + level: Option, + src: &mut LintLevelSource, sess: &Session, - ) -> LevelAndSource { - let (level, mut src) = self.get_lint_id_level(LintId::of(lint), idx, aux); - + lint: &'static Lint, + get_lint_id_level: impl FnOnce(LintId) -> (Option, LintLevelSource), + ) -> Level { // If `level` is none then we actually assume the default level for this // lint. let mut level = level.unwrap_or_else(|| lint.default_level(sess.edition())); @@ -101,12 +107,11 @@ impl LintLevelSets { // and so if we turned that into an error, it'd defeat the purpose of the // future compatibility warning. if level == Level::Warn && LintId::of(lint) != LintId::of(FORBIDDEN_LINT_GROUPS) { - let (warnings_level, warnings_src) = - self.get_lint_id_level(LintId::of(builtin::WARNINGS), idx, aux); + let (warnings_level, warnings_src) = get_lint_id_level(LintId::of(builtin::WARNINGS)); if let Some(configured_warning_level) = warnings_level { if configured_warning_level != Level::Warn { level = configured_warning_level; - src = warnings_src; + *src = warnings_src; } } } @@ -116,7 +121,7 @@ impl LintLevelSets { level = if let LintLevelSource::CommandLine(_, Level::ForceWarn(_)) = src { level } else { - cmp::min(level, self.lint_cap) + cmp::min(level, sess.opts.lint_cap.unwrap_or(Level::Forbid)) }; if let Some(driver_level) = sess.driver_lint_caps.get(&LintId::of(lint)) { @@ -124,6 +129,22 @@ impl LintLevelSets { level = cmp::min(*driver_level, level); } + level + } + + pub fn get_lint_level( + &self, + lint: &'static Lint, + idx: LintStackIndex, + aux: Option<&FxHashMap>, + sess: &Session, + ) -> LevelAndSource { + let (level, mut src) = self.get_lint_id_level(LintId::of(lint), idx, aux); + + let level = Self::actual_level(level, &mut src, sess, lint, |id| { + self.get_lint_id_level(id, idx, aux) + }); + (level, src) } @@ -193,6 +214,58 @@ impl<'a> HashStable> for LintLevelMap { hcx.while_hashing_spans(true, |hcx| sets.hash_stable(hcx, hasher)) } } +pub struct LintLevelQueryMap<'tcx> { + pub tcx: TyCtxt<'tcx>, + pub cur: HirId, + pub specs: FxHashMap, +} + +impl<'tcx> LintLevelQueryMap<'tcx> { + pub fn lint_id_level(&self, id: LintId) -> (Option, LintLevelSource) { + Self::get_lint_id_level(id, self.cur, self.tcx, &self.specs) + } + + pub fn lint_level(&self, lint: &'static Lint) -> LevelAndSource { + Self::get_lint_level(LintId::of(lint), self.cur, self.tcx, &self.specs) + } + + pub fn get_lint_id_level( + id: LintId, + cur: HirId, + tcx: TyCtxt<'tcx>, + specs: &FxHashMap, + ) -> (Option, LintLevelSource) { + if let Some(&(level, src)) = specs.get(&id) { + return (Some(level), src); + } + let mut cur = cur; + + loop { + let parent = tcx.hir().get_parent_node(cur); + if cur == parent { + return (None, LintLevelSource::Default); + } + let specs = tcx.lint_levels_on(parent); + if let Some(&(level, src)) = specs.get(&id) { + return (Some(level), src); + } + cur = parent + } + } + + pub fn get_lint_level( + id: LintId, + cur: HirId, + tcx: TyCtxt<'tcx>, + specs: &FxHashMap, + ) -> (Level, LintLevelSource) { + let (level, mut src) = Self::get_lint_id_level(id, cur, tcx, specs); + let level = LintLevelSets::actual_level(level, &mut src, tcx.sess, id.lint, |id| { + Self::get_lint_id_level(id, cur, tcx, specs) + }); + (level, src) + } +} /// This struct represents a lint expectation and holds all required information /// to emit the `unfulfilled_lint_expectations` lint if it is unfulfilled after @@ -261,11 +334,11 @@ pub fn explain_lint_level_source( )); } } - LintLevelSource::Node(lint_attr_name, src, reason) => { + LintLevelSource::Node { name: lint_attr_name, span, reason, .. } => { if let Some(rationale) = reason { err.note(rationale.as_str()); } - err.span_note_once(src, "the lint level is defined here"); + err.span_note_once(span, "the lint level is defined here"); if lint_attr_name.as_str() != name { let level_str = level.as_str(); err.note_once(&format!( diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 9cb19583121ec..09c336f9e4c7e 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -273,10 +273,14 @@ rustc_queries! { separate_provide_extern } - query lint_levels(_: ()) -> LintLevelMap { + query lint_levels_on(key: HirId) -> FxHashMap { arena_cache - eval_always - desc { "computing the lint levels for items in this crate" } + desc { |tcx| "looking up lint levels for `{}`", key } + } + + query lint_expectations(_: ()) -> Vec<(LintExpectationId, LintExpectation)> { + arena_cache + desc { "computing `#[expect]`ed lints in this crate" } } query parent_module_from_def_id(key: LocalDefId) -> LocalDefId { diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 94eddef944a02..4cd9004fa1845 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -53,7 +53,7 @@ use rustc_query_system::ich::StableHashingContext; use rustc_serialize::opaque::{FileEncodeResult, FileEncoder}; use rustc_session::config::{CrateType, OutputFilenames}; use rustc_session::cstore::CrateStoreDyn; -use rustc_session::lint::{Level, Lint}; +use rustc_session::lint::{Level, Lint, LintId}; use rustc_session::Limit; use rustc_session::Session; use rustc_span::def_id::{DefPathHash, StableCrateId}; @@ -2832,19 +2832,16 @@ impl<'tcx> TyCtxt<'tcx> { pub fn lint_level_at_node( self, lint: &'static Lint, - mut id: hir::HirId, + id: hir::HirId, ) -> (Level, LintLevelSource) { - let sets = self.lint_levels(()); - loop { - if let Some(pair) = sets.level_and_source(lint, id, self.sess) { - return pair; - } - let next = self.hir().get_parent_node(id); - if next == id { - bug!("lint traversal reached the root of the crate"); - } - id = next; - } + let level_and_src = crate::lint::LintLevelQueryMap::get_lint_level( + LintId::of(lint), + id, + self, + self.lint_levels_on(id), + ); + debug!(?id, ?level_and_src); + level_and_src } /// Emit a lint at `span` from a lint struct (some type that implements `DecorateLint`, diff --git a/compiler/rustc_middle/src/ty/query.rs b/compiler/rustc_middle/src/ty/query.rs index a300a8df23d28..1a1b020905bf6 100644 --- a/compiler/rustc_middle/src/ty/query.rs +++ b/compiler/rustc_middle/src/ty/query.rs @@ -1,6 +1,6 @@ use crate::dep_graph; use crate::infer::canonical::{self, Canonical}; -use crate::lint::LintLevelMap; +use crate::lint::{LevelAndSource, LintExpectation}; use crate::metadata::ModChild; use crate::middle::codegen_fn_attrs::CodegenFnAttrs; use crate::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo}; @@ -44,12 +44,14 @@ use rustc_errors::ErrorGuaranteed; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, DefIdSet, LocalDefId}; +use rustc_hir::hir_id::HirId; use rustc_hir::lang_items::{LangItem, LanguageItems}; use rustc_hir::{Crate, ItemLocalId, TraitCandidate}; use rustc_index::{bit_set::FiniteBitSet, vec::IndexVec}; use rustc_session::config::{EntryFnType, OptLevel, OutputFilenames, SymbolManglingVersion}; use rustc_session::cstore::{CrateDepKind, CrateSource}; use rustc_session::cstore::{ExternCrate, ForeignModule, LinkagePreference, NativeLib}; +use rustc_session::lint::{LintExpectationId, LintId}; use rustc_session::utils::NativeLibKind; use rustc_session::Limits; use rustc_span::symbol::Symbol; diff --git a/compiler/rustc_query_impl/src/keys.rs b/compiler/rustc_query_impl/src/keys.rs index 49175e97f4171..ea2d10cd14bf7 100644 --- a/compiler/rustc_query_impl/src/keys.rs +++ b/compiler/rustc_query_impl/src/keys.rs @@ -1,6 +1,7 @@ //! Defines the set of legal keys that can be used in queries. use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE}; +use rustc_hir::hir_id::HirId; use rustc_middle::infer::canonical::Canonical; use rustc_middle::mir; use rustc_middle::traits; @@ -543,3 +544,19 @@ impl<'tcx> Key for (Ty<'tcx>, ty::ValTree<'tcx>) { DUMMY_SP } } + +impl Key for HirId { + #[inline(always)] + fn query_crate_is_local(&self) -> bool { + true + } + + fn default_span(&self, tcx: TyCtxt<'_>) -> Span { + self.owner.default_span(tcx) + } + + #[inline(always)] + fn key_as_def_id(&self) -> Option { + Some(self.owner.to_def_id()) + } +} diff --git a/src/test/ui/lint/rfc-2383-lint-reason/force_warn_expected_lints_fulfilled.stderr b/src/test/ui/lint/rfc-2383-lint-reason/force_warn_expected_lints_fulfilled.stderr index 06befcbb5117e..5942fa8aeb4f0 100644 --- a/src/test/ui/lint/rfc-2383-lint-reason/force_warn_expected_lints_fulfilled.stderr +++ b/src/test/ui/lint/rfc-2383-lint-reason/force_warn_expected_lints_fulfilled.stderr @@ -1,11 +1,3 @@ -warning: denote infinite loops with `loop { ... }` - --> $DIR/force_warn_expected_lints_fulfilled.rs:10: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:20:9 | @@ -36,5 +28,13 @@ LL | let mut what_does_the_fox_say = "*ding* *deng* *dung*"; | = note: requested on the command line with `--force-warn unused-mut` +warning: denote infinite loops with `loop { ... }` + --> $DIR/force_warn_expected_lints_fulfilled.rs:10:5 + | +LL | while true { + | ^^^^^^^^^^ help: use `loop` + | + = note: requested on the command line with `--force-warn while-true` + warning: 5 warnings emitted From bd45139cb014d51adaff46b9797504bc6eac2564 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 9 Sep 2022 12:05:01 +0200 Subject: [PATCH 2/8] Allow query system to recover a HirId. --- .../rustc_middle/src/dep_graph/dep_node.rs | 38 ++++++++++++++++--- .../rustc_query_system/src/dep_graph/mod.rs | 6 ++- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index 7718906ac4ee7..aae12705e3d43 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -62,7 +62,7 @@ use crate::ty::TyCtxt; use rustc_data_structures::fingerprint::Fingerprint; use rustc_hir::def_id::{CrateNum, DefId, LocalDefId}; use rustc_hir::definitions::DefPathHash; -use rustc_hir::HirId; +use rustc_hir::{HirId, ItemLocalId}; use rustc_query_system::dep_graph::FingerprintStyle; use rustc_span::symbol::Symbol; use std::hash::Hash; @@ -289,7 +289,7 @@ impl DepNodeExt for DepNode { let kind = dep_kind_from_label_string(label)?; match kind.fingerprint_style(tcx) { - FingerprintStyle::Opaque => Err(()), + FingerprintStyle::Opaque | FingerprintStyle::HirId => Err(()), FingerprintStyle::Unit => Ok(DepNode::new_no_params(tcx, kind)), FingerprintStyle::DefPathHash => { Ok(DepNode::from_def_path_hash(tcx, def_path_hash, kind)) @@ -417,7 +417,7 @@ impl<'tcx> DepNodeParams> for (DefId, DefId) { impl<'tcx> DepNodeParams> for HirId { #[inline(always)] fn fingerprint_style() -> FingerprintStyle { - FingerprintStyle::Opaque + FingerprintStyle::HirId } // We actually would not need to specialize the implementation of this @@ -426,10 +426,36 @@ impl<'tcx> DepNodeParams> for HirId { #[inline(always)] fn to_fingerprint(&self, tcx: TyCtxt<'tcx>) -> Fingerprint { let HirId { owner, local_id } = *self; - let def_path_hash = tcx.def_path_hash(owner.to_def_id()); - let local_id = Fingerprint::from_smaller_hash(local_id.as_u32().into()); + Fingerprint::new( + // `owner` is local, so is completely defined by the local hash + def_path_hash.local_hash(), + local_id.as_u32().into(), + ) + } - def_path_hash.0.combine(local_id) + #[inline(always)] + fn to_debug_str(&self, tcx: TyCtxt<'tcx>) -> String { + let HirId { owner, local_id } = *self; + format!("{}.{}", tcx.def_path_str(owner.to_def_id()), local_id.as_u32()) + } + + #[inline(always)] + fn recover(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> Option { + if dep_node.kind.fingerprint_style(tcx) == FingerprintStyle::HirId { + let (local_hash, local_id) = Fingerprint::from(dep_node.hash).as_value(); + let def_path_hash = DefPathHash::new(tcx.sess.local_stable_crate_id(), local_hash); + let owner = tcx + .def_path_hash_to_def_id(def_path_hash, &mut || { + panic!("Failed to extract HirId: {:?} {}", dep_node.kind, dep_node.hash) + }) + .expect_local(); + let local_id = local_id + .try_into() + .unwrap_or_else(|_| panic!("local id should be u32, found {:?}", local_id)); + Some(HirId { owner, local_id: ItemLocalId::from_u32(local_id) }) + } else { + None + } } } diff --git a/compiler/rustc_query_system/src/dep_graph/mod.rs b/compiler/rustc_query_system/src/dep_graph/mod.rs index 342d95ca490ea..6f3bd7762165c 100644 --- a/compiler/rustc_query_system/src/dep_graph/mod.rs +++ b/compiler/rustc_query_system/src/dep_graph/mod.rs @@ -67,6 +67,8 @@ impl HasDepContext for T { pub enum FingerprintStyle { /// The fingerprint is actually a DefPathHash. DefPathHash, + /// The fingerprint is actually a HirId. + HirId, /// Query key was `()` or equivalent, so fingerprint is just zero. Unit, /// Some opaque hash. @@ -77,7 +79,9 @@ impl FingerprintStyle { #[inline] pub fn reconstructible(self) -> bool { match self { - FingerprintStyle::DefPathHash | FingerprintStyle::Unit => true, + FingerprintStyle::DefPathHash | FingerprintStyle::Unit | FingerprintStyle::HirId => { + true + } FingerprintStyle::Opaque => false, } } From ad09abc19415d48e24dbb231c85e521d909e77f7 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 10 Sep 2022 01:28:08 +0200 Subject: [PATCH 3/8] Move some code and add comments. --- compiler/rustc_lint/src/levels.rs | 135 +++++++++--- compiler/rustc_lint/src/lib.rs | 1 + compiler/rustc_middle/src/lint.rs | 263 ++++++++---------------- compiler/rustc_middle/src/query/mod.rs | 2 +- compiler/rustc_middle/src/ty/context.rs | 39 +--- compiler/rustc_middle/src/ty/query.rs | 4 +- 6 files changed, 192 insertions(+), 252 deletions(-) diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index 1626a4a99df79..9281d57ac64b4 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -6,10 +6,11 @@ use rustc_data_structures::fx::FxHashMap; use rustc_errors::{Applicability, Diagnostic, LintDiagnosticBuilder, MultiSpan}; use rustc_hir as hir; use rustc_hir::{intravisit, HirId}; +use rustc_index::vec::IndexVec; use rustc_middle::hir::nested_filter; use rustc_middle::lint::{ - struct_lint_level, LevelAndSource, LintExpectation, LintLevelQueryMap, LintLevelSets, - LintLevelSource, LintSet, LintStackIndex, COMMAND_LINE, + reveal_actual_level, struct_lint_level, LevelAndSource, LintExpectation, LintLevelSource, + ShallowLintLevelMap, }; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{RegisteredTools, TyCtxt}; @@ -27,13 +28,80 @@ use crate::errors::{ UnknownToolInScopedLint, }; +#[derive(Debug)] +struct LintLevelSets { + list: IndexVec, +} + +rustc_index::newtype_index! { + struct LintStackIndex { + ENCODABLE = custom, // we don't need encoding + const COMMAND_LINE = 0, + } +} + +#[derive(Debug)] +struct LintSet { + // -A,-W,-D flags, a `Symbol` for the flag itself and `Level` for which + // flag. + specs: FxHashMap, + + parent: LintStackIndex, +} + +impl LintLevelSets { + fn new() -> Self { + LintLevelSets { list: IndexVec::new() } + } + + fn get_lint_level( + &self, + lint: &'static Lint, + idx: LintStackIndex, + aux: Option<&FxHashMap>, + sess: &Session, + ) -> LevelAndSource { + let lint = LintId::of(lint); + let (level, mut src) = self.raw_lint_id_level(lint, idx, aux); + let level = reveal_actual_level(level, &mut src, sess, lint, |id| { + self.raw_lint_id_level(id, idx, aux) + }); + (level, src) + } + + fn raw_lint_id_level( + &self, + id: LintId, + mut idx: LintStackIndex, + aux: Option<&FxHashMap>, + ) -> (Option, LintLevelSource) { + if let Some(specs) = aux { + if let Some(&(level, src)) = specs.get(&id) { + return (Some(level), src); + } + } + loop { + let LintSet { ref specs, parent } = self.list[idx]; + if let Some(&(level, src)) = specs.get(&id) { + return (Some(level), src); + } + if idx == COMMAND_LINE { + return (None, LintLevelSource::Default); + } + idx = parent; + } + } +} + fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExpectation)> { let store = unerased_lint_store(tcx); let mut builder = LintLevelsBuilder { sess: tcx.sess, provider: QueryMapExpectationsWrapper { - map: LintLevelQueryMap { tcx, cur: hir::CRATE_HIR_ID, specs: FxHashMap::default() }, + tcx, + cur: hir::CRATE_HIR_ID, + specs: ShallowLintLevelMap::default(), expectations: Vec::new(), unstable_to_stable_ids: FxHashMap::default(), }, @@ -51,12 +119,12 @@ fn lint_expectations(tcx: TyCtxt<'_>, (): ()) -> Vec<(LintExpectationId, LintExp builder.provider.expectations } -fn lint_levels_on(tcx: TyCtxt<'_>, hir_id: HirId) -> FxHashMap { +fn shallow_lint_levels_on(tcx: TyCtxt<'_>, hir_id: HirId) -> ShallowLintLevelMap { let store = unerased_lint_store(tcx); let mut levels = LintLevelsBuilder { sess: tcx.sess, - provider: LintLevelQueryMap { tcx, cur: hir_id, specs: FxHashMap::default() }, + provider: LintLevelQueryMap { tcx, cur: hir_id, specs: ShallowLintLevelMap::default() }, warn_about_weird_lints: false, store, registered_tools: &tcx.resolutions(()).registered_tools, @@ -77,12 +145,6 @@ pub struct TopDown { cur: LintStackIndex, } -pub struct QueryMapExpectationsWrapper<'tcx> { - map: LintLevelQueryMap<'tcx>, - expectations: Vec<(LintExpectationId, LintExpectation)>, - unstable_to_stable_ids: FxHashMap, -} - pub trait LintLevelsProvider { fn current_specs(&self) -> &FxHashMap; fn current_specs_mut(&mut self) -> &mut FxHashMap; @@ -104,28 +166,42 @@ impl LintLevelsProvider for TopDown { } } +struct LintLevelQueryMap<'tcx> { + tcx: TyCtxt<'tcx>, + cur: HirId, + specs: ShallowLintLevelMap, +} + impl LintLevelsProvider for LintLevelQueryMap<'_> { fn current_specs(&self) -> &FxHashMap { - &self.specs + &self.specs.specs } fn current_specs_mut(&mut self) -> &mut FxHashMap { - &mut self.specs + &mut self.specs.specs } fn get_lint_level(&self, lint: &'static Lint, _: &Session) -> LevelAndSource { - self.lint_level(lint) + self.specs.lint_level_id_at_node(self.tcx, LintId::of(lint), self.cur) } } +struct QueryMapExpectationsWrapper<'tcx> { + tcx: TyCtxt<'tcx>, + cur: HirId, + specs: ShallowLintLevelMap, + expectations: Vec<(LintExpectationId, LintExpectation)>, + unstable_to_stable_ids: FxHashMap, +} + impl LintLevelsProvider for QueryMapExpectationsWrapper<'_> { fn current_specs(&self) -> &FxHashMap { - &self.map.specs + &self.specs.specs } fn current_specs_mut(&mut self) -> &mut FxHashMap { - self.map.specs.clear(); - &mut self.map.specs + self.specs.specs.clear(); + &mut self.specs.specs } fn get_lint_level(&self, lint: &'static Lint, _: &Session) -> LevelAndSource { - self.map.lint_level(lint) + 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") }; @@ -144,11 +220,7 @@ impl LintLevelsProvider for QueryMapExpectationsWrapper<'_> { impl<'tcx> LintLevelsBuilder<'_, QueryMapExpectationsWrapper<'tcx>> { fn add_id(&mut self, hir_id: HirId) { - self.add( - self.provider.map.tcx.hir().attrs(hir_id), - hir_id == hir::CRATE_HIR_ID, - Some(hir_id), - ); + self.add(self.provider.tcx.hir().attrs(hir_id), hir_id == hir::CRATE_HIR_ID, Some(hir_id)); } } @@ -156,7 +228,7 @@ impl<'tcx> intravisit::Visitor<'tcx> for LintLevelsBuilder<'_, QueryMapExpectati type NestedFilter = nested_filter::All; fn nested_visit_map(&mut self) -> Self::Map { - self.provider.map.tcx.hir() + self.provider.tcx.hir() } fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) { @@ -225,13 +297,12 @@ pub struct LintLevelsBuilder<'s, P> { registered_tools: &'s RegisteredTools, } -pub struct BuilderPush { +pub(crate) struct BuilderPush { prev: LintStackIndex, - pub changed: bool, } impl<'s> LintLevelsBuilder<'s, TopDown> { - pub fn new( + pub(crate) fn new( sess: &'s Session, warn_about_weird_lints: bool, store: &'s LintStore, @@ -289,11 +360,11 @@ impl<'s> LintLevelsBuilder<'s, TopDown> { self.provider.cur = prev; } - BuilderPush { prev, changed: prev != self.provider.cur } + BuilderPush { prev } } /// Called after `push` when the scope of a set of attributes are exited. - pub fn pop(&mut self, push: BuilderPush) { + pub(crate) fn pop(&mut self, push: BuilderPush) { self.provider.cur = push.prev; } } @@ -855,7 +926,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { /// Used to emit a lint-related diagnostic based on the current state of /// this lint context. - pub fn struct_lint( + pub(crate) fn struct_lint( &self, lint: &'static Lint, span: Option, @@ -866,6 +937,6 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { } } -pub fn provide(providers: &mut Providers) { - *providers = Providers { lint_levels_on, lint_expectations, ..*providers }; +pub(crate) fn provide(providers: &mut Providers) { + *providers = Providers { shallow_lint_levels_on, lint_expectations, ..*providers }; } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index e0e6f1a8c080c..2a22a6e5be1ba 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -35,6 +35,7 @@ #![feature(iter_order_by)] #![feature(let_chains)] #![feature(let_else)] +#![feature(min_specialization)] #![feature(never_type)] #![recursion_limit = "256"] diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index c659e677fa0e1..c43a4b7a42eea 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -1,14 +1,11 @@ use std::cmp; use rustc_data_structures::fx::FxHashMap; -use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_errors::{Diagnostic, DiagnosticId, LintDiagnosticBuilder, MultiSpan}; use rustc_hir::HirId; -use rustc_index::vec::IndexVec; -use rustc_query_system::ich::StableHashingContext; use rustc_session::lint::{ builtin::{self, FORBIDDEN_LINT_GROUPS}, - FutureIncompatibilityReason, Level, Lint, LintExpectationId, LintId, + FutureIncompatibilityReason, Level, Lint, LintId, }; use rustc_session::Session; use rustc_span::hygiene::MacroKind; @@ -61,209 +58,115 @@ impl LintLevelSource { /// A tuple of a lint level and its source. pub type LevelAndSource = (Level, LintLevelSource); -#[derive(Debug, HashStable)] -pub struct LintLevelSets { - pub list: IndexVec, -} - -rustc_index::newtype_index! { - #[derive(HashStable)] - pub struct LintStackIndex { - const COMMAND_LINE = 0, - } -} - -#[derive(Debug, HashStable)] -pub struct LintSet { - // -A,-W,-D flags, a `Symbol` for the flag itself and `Level` for which - // flag. +/// Return type for the `shallow_lint_levels_on` query. +/// +/// This map represents the set of allowed lints and allowance levels given +/// by the attributes for *a single HirId*. +#[derive(Default, Debug, HashStable)] +pub struct ShallowLintLevelMap { pub specs: FxHashMap, - - pub parent: LintStackIndex, } -impl LintLevelSets { - pub fn new() -> Self { - LintLevelSets { list: IndexVec::new() } - } - - pub fn actual_level( - level: Option, - src: &mut LintLevelSource, - sess: &Session, - lint: &'static Lint, - get_lint_id_level: impl FnOnce(LintId) -> (Option, LintLevelSource), - ) -> Level { - // If `level` is none then we actually assume the default level for this - // lint. - let mut level = level.unwrap_or_else(|| lint.default_level(sess.edition())); - - // If we're about to issue a warning, check at the last minute for any - // directives against the warnings "lint". If, for example, there's an - // `allow(warnings)` in scope then we want to respect that instead. - // - // We exempt `FORBIDDEN_LINT_GROUPS` from this because it specifically - // triggers in cases (like #80988) where you have `forbid(warnings)`, - // and so if we turned that into an error, it'd defeat the purpose of the - // future compatibility warning. - if level == Level::Warn && LintId::of(lint) != LintId::of(FORBIDDEN_LINT_GROUPS) { - let (warnings_level, warnings_src) = get_lint_id_level(LintId::of(builtin::WARNINGS)); - if let Some(configured_warning_level) = warnings_level { - if configured_warning_level != Level::Warn { - level = configured_warning_level; - *src = warnings_src; - } +/// From an initial level and source, verify the effect of special annotations: +/// `warnings` lint level and lint caps. +/// +/// The return of this function is suitable for diagnostics. +pub fn reveal_actual_level( + level: Option, + src: &mut LintLevelSource, + sess: &Session, + lint: LintId, + probe_for_lint_level: impl FnOnce(LintId) -> (Option, LintLevelSource), +) -> Level { + // If `level` is none then we actually assume the default level for this lint. + let mut level = level.unwrap_or_else(|| lint.lint.default_level(sess.edition())); + + // If we're about to issue a warning, check at the last minute for any + // directives against the warnings "lint". If, for example, there's an + // `allow(warnings)` in scope then we want to respect that instead. + // + // We exempt `FORBIDDEN_LINT_GROUPS` from this because it specifically + // triggers in cases (like #80988) where you have `forbid(warnings)`, + // and so if we turned that into an error, it'd defeat the purpose of the + // future compatibility warning. + if level == Level::Warn && lint != LintId::of(FORBIDDEN_LINT_GROUPS) { + let (warnings_level, warnings_src) = probe_for_lint_level(LintId::of(builtin::WARNINGS)); + if let Some(configured_warning_level) = warnings_level { + if configured_warning_level != Level::Warn { + level = configured_warning_level; + *src = warnings_src; } } - - // Ensure that we never exceed the `--cap-lints` argument - // unless the source is a --force-warn - level = if let LintLevelSource::CommandLine(_, Level::ForceWarn(_)) = src { - level - } else { - cmp::min(level, sess.opts.lint_cap.unwrap_or(Level::Forbid)) - }; - - if let Some(driver_level) = sess.driver_lint_caps.get(&LintId::of(lint)) { - // Ensure that we never exceed driver level. - level = cmp::min(*driver_level, level); - } - - level } - pub fn get_lint_level( - &self, - lint: &'static Lint, - idx: LintStackIndex, - aux: Option<&FxHashMap>, - sess: &Session, - ) -> LevelAndSource { - let (level, mut src) = self.get_lint_id_level(LintId::of(lint), idx, aux); - - let level = Self::actual_level(level, &mut src, sess, lint, |id| { - self.get_lint_id_level(id, idx, aux) - }); + // Ensure that we never exceed the `--cap-lints` argument unless the source is a --force-warn + level = if let LintLevelSource::CommandLine(_, Level::ForceWarn(_)) = src { + level + } else { + cmp::min(level, sess.opts.lint_cap.unwrap_or(Level::Forbid)) + }; - (level, src) + if let Some(driver_level) = sess.driver_lint_caps.get(&lint) { + // Ensure that we never exceed driver level. + level = cmp::min(*driver_level, level); } - pub fn get_lint_id_level( + level +} + +impl ShallowLintLevelMap { + /// Perform a deep probe in the HIR tree looking for the actual level for the lint. + /// This lint level is not usable for diagnostics, it needs to be corrected by + /// `reveal_actual_level` beforehand. + fn probe_for_lint_level( &self, + tcx: TyCtxt<'_>, id: LintId, - mut idx: LintStackIndex, - aux: Option<&FxHashMap>, + start: HirId, ) -> (Option, LintLevelSource) { - if let Some(specs) = aux { - if let Some(&(level, src)) = specs.get(&id) { - return (Some(level), src); - } + if let Some(&(level, src)) = self.specs.get(&id) { + return (Some(level), src); } - loop { - let LintSet { ref specs, parent } = self.list[idx]; - if let Some(&(level, src)) = specs.get(&id) { + + for (parent, _) in tcx.hir().parent_iter(start) { + let specs = tcx.shallow_lint_levels_on(parent); + if let Some(&(level, src)) = specs.specs.get(&id) { return (Some(level), src); } - if idx == COMMAND_LINE { - return (None, LintLevelSource::Default); - } - idx = parent; } + (None, LintLevelSource::Default) } -} - -#[derive(Debug)] -pub struct LintLevelMap { - /// This is a collection of lint expectations as described in RFC 2383, that - /// can be fulfilled during this compilation session. This means that at least - /// one expected lint is currently registered in the lint store. - /// - /// The [`LintExpectationId`] is stored as a part of the [`Expect`](Level::Expect) - /// lint level. - pub lint_expectations: Vec<(LintExpectationId, LintExpectation)>, - pub sets: LintLevelSets, - pub id_to_set: FxHashMap, -} -impl LintLevelMap { - /// If the `id` was previously registered with `register_id` when building - /// this `LintLevelMap` this returns the corresponding lint level and source - /// of the lint level for the lint provided. - /// - /// If the `id` was not previously registered, returns `None`. If `None` is - /// returned then the parent of `id` should be acquired and this function - /// should be called again. - pub fn level_and_source( + /// Fetch and return the user-visible lint level for the given lint at the given HirId. + pub fn lint_level_id_at_node( &self, - lint: &'static Lint, + tcx: TyCtxt<'_>, + lint: LintId, id: HirId, - session: &Session, - ) -> Option { - self.id_to_set.get(&id).map(|idx| self.sets.get_lint_level(lint, *idx, None, session)) - } -} - -impl<'a> HashStable> for LintLevelMap { - #[inline] - fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { - let LintLevelMap { ref sets, ref id_to_set, ref lint_expectations } = *self; - - id_to_set.hash_stable(hcx, hasher); - lint_expectations.hash_stable(hcx, hasher); - - hcx.while_hashing_spans(true, |hcx| sets.hash_stable(hcx, hasher)) + ) -> (Level, LintLevelSource) { + let (level, mut src) = self.probe_for_lint_level(tcx, lint, id); + let level = reveal_actual_level(level, &mut src, tcx.sess, lint, |lint| { + self.probe_for_lint_level(tcx, lint, id) + }); + debug!(?id, ?level, ?src); + (level, src) } } -pub struct LintLevelQueryMap<'tcx> { - pub tcx: TyCtxt<'tcx>, - pub cur: HirId, - pub specs: FxHashMap, -} - -impl<'tcx> LintLevelQueryMap<'tcx> { - pub fn lint_id_level(&self, id: LintId) -> (Option, LintLevelSource) { - Self::get_lint_id_level(id, self.cur, self.tcx, &self.specs) - } - pub fn lint_level(&self, lint: &'static Lint) -> LevelAndSource { - Self::get_lint_level(LintId::of(lint), self.cur, self.tcx, &self.specs) +impl TyCtxt<'_> { + /// Fetch and return the user-visible lint level for the given lint at the given HirId. + pub fn lint_level_at_node(self, lint: &'static Lint, id: HirId) -> (Level, LintLevelSource) { + self.shallow_lint_levels_on(id).lint_level_id_at_node(self, LintId::of(lint), id) } - pub fn get_lint_id_level( - id: LintId, - cur: HirId, - tcx: TyCtxt<'tcx>, - specs: &FxHashMap, - ) -> (Option, LintLevelSource) { - if let Some(&(level, src)) = specs.get(&id) { - return (Some(level), src); + /// Walks upwards from `id` to find a node which might change lint levels with attributes. + /// It stops at `bound` and just returns it if reached. + pub fn maybe_lint_level_root_bounded(self, mut id: HirId, bound: HirId) -> HirId { + let hir = self.hir(); + while id != bound && self.shallow_lint_levels_on(id).specs.is_empty() { + id = hir.get_parent_node(id) } - let mut cur = cur; - - loop { - let parent = tcx.hir().get_parent_node(cur); - if cur == parent { - return (None, LintLevelSource::Default); - } - let specs = tcx.lint_levels_on(parent); - if let Some(&(level, src)) = specs.get(&id) { - return (Some(level), src); - } - cur = parent - } - } - - pub fn get_lint_level( - id: LintId, - cur: HirId, - tcx: TyCtxt<'tcx>, - specs: &FxHashMap, - ) -> (Level, LintLevelSource) { - let (level, mut src) = Self::get_lint_id_level(id, cur, tcx, specs); - let level = LintLevelSets::actual_level(level, &mut src, tcx.sess, id.lint, |id| { - Self::get_lint_id_level(id, cur, tcx, specs) - }); - (level, src) + id } } diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 09c336f9e4c7e..312777230aaea 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -273,7 +273,7 @@ rustc_queries! { separate_provide_extern } - query lint_levels_on(key: HirId) -> FxHashMap { + query shallow_lint_levels_on(key: HirId) -> rustc_middle::lint::ShallowLintLevelMap { arena_cache desc { |tcx| "looking up lint levels for `{}`", key } } diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 4cd9004fa1845..50d6dcbb79f20 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -4,7 +4,7 @@ use crate::arena::Arena; use crate::dep_graph::{DepGraph, DepKind, DepKindStruct}; use crate::hir::place::Place as HirPlace; use crate::infer::canonical::{Canonical, CanonicalVarInfo, CanonicalVarInfos}; -use crate::lint::{struct_lint_level, LintLevelSource}; +use crate::lint::struct_lint_level; use crate::middle::codegen_fn_attrs::CodegenFnAttrs; use crate::middle::resolve_lifetime; use crate::middle::stability; @@ -53,7 +53,7 @@ use rustc_query_system::ich::StableHashingContext; use rustc_serialize::opaque::{FileEncodeResult, FileEncoder}; use rustc_session::config::{CrateType, OutputFilenames}; use rustc_session::cstore::CrateStoreDyn; -use rustc_session::lint::{Level, Lint, LintId}; +use rustc_session::lint::Lint; use rustc_session::Limit; use rustc_session::Session; use rustc_span::def_id::{DefPathHash, StableCrateId}; @@ -2809,41 +2809,6 @@ impl<'tcx> TyCtxt<'tcx> { iter.intern_with(|xs| self.intern_bound_variable_kinds(xs)) } - /// Walks upwards from `id` to find a node which might change lint levels with attributes. - /// It stops at `bound` and just returns it if reached. - pub fn maybe_lint_level_root_bounded(self, mut id: HirId, bound: HirId) -> HirId { - let hir = self.hir(); - loop { - if id == bound { - return bound; - } - - if hir.attrs(id).iter().any(|attr| Level::from_attr(attr).is_some()) { - return id; - } - let next = hir.get_parent_node(id); - if next == id { - bug!("lint traversal reached the root of the crate"); - } - id = next; - } - } - - pub fn lint_level_at_node( - self, - lint: &'static Lint, - id: hir::HirId, - ) -> (Level, LintLevelSource) { - let level_and_src = crate::lint::LintLevelQueryMap::get_lint_level( - LintId::of(lint), - id, - self, - self.lint_levels_on(id), - ); - debug!(?id, ?level_and_src); - level_and_src - } - /// Emit a lint at `span` from a lint struct (some type that implements `DecorateLint`, /// typically generated by `#[derive(LintDiagnostic)]`). pub fn emit_spanned_lint( diff --git a/compiler/rustc_middle/src/ty/query.rs b/compiler/rustc_middle/src/ty/query.rs index 1a1b020905bf6..36ea7946e9ae4 100644 --- a/compiler/rustc_middle/src/ty/query.rs +++ b/compiler/rustc_middle/src/ty/query.rs @@ -1,6 +1,6 @@ use crate::dep_graph; use crate::infer::canonical::{self, Canonical}; -use crate::lint::{LevelAndSource, LintExpectation}; +use crate::lint::LintExpectation; use crate::metadata::ModChild; use crate::middle::codegen_fn_attrs::CodegenFnAttrs; use crate::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo}; @@ -51,7 +51,7 @@ use rustc_index::{bit_set::FiniteBitSet, vec::IndexVec}; use rustc_session::config::{EntryFnType, OptLevel, OutputFilenames, SymbolManglingVersion}; use rustc_session::cstore::{CrateDepKind, CrateSource}; use rustc_session::cstore::{ExternCrate, ForeignModule, LinkagePreference, NativeLib}; -use rustc_session::lint::{LintExpectationId, LintId}; +use rustc_session::lint::LintExpectationId; use rustc_session::utils::NativeLibKind; use rustc_session::Limits; use rustc_span::symbol::Symbol; From bb6184204813a83f6e39a3141b4cb9248237c8ed Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 10 Sep 2022 09:58:35 +0200 Subject: [PATCH 4/8] Remove unused tool_name. --- compiler/rustc_lint/src/levels.rs | 4 ---- compiler/rustc_middle/src/lint.rs | 2 -- 2 files changed, 6 deletions(-) diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index 9281d57ac64b4..eef342d4b07a2 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -692,7 +692,6 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { .name, span: sp, reason, - tool: tool_name, }; for &id in *ids { if self.check_gated_lint(id, attr.span) { @@ -710,7 +709,6 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { name: Symbol::intern(complete_name), span: sp, reason, - tool: tool_name, }; for &id in ids { if self.check_gated_lint(id, attr.span) { @@ -754,7 +752,6 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { name: Symbol::intern(&new_lint_name), span: sp, reason, - tool: tool_name, }; for id in ids { self.insert_spec(*id, (level, src)); @@ -845,7 +842,6 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { name: Symbol::intern(&new_name), span: sp, reason, - tool: tool_name, }; for &id in ids { if self.check_gated_lint(id, attr.span) { diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index c43a4b7a42eea..4cb5ef7917774 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -27,8 +27,6 @@ pub enum LintLevelSource { span: Span, /// RFC 2383 reason reason: Option, - /// The lint tool. (e.g. rustdoc, clippy) - tool: Option, }, /// Lint level was set by a command-line flag. From 69613bb60223c97fa994ebb00fc789922cd0a483 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 10 Sep 2022 09:58:49 +0200 Subject: [PATCH 5/8] Bless ui test. --- ...te-non_exhaustive_omitted_patterns_lint.rs | 5 ++ ...on_exhaustive_omitted_patterns_lint.stderr | 72 ++++++++++++++++--- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/src/test/ui/feature-gates/feature-gate-non_exhaustive_omitted_patterns_lint.rs b/src/test/ui/feature-gates/feature-gate-non_exhaustive_omitted_patterns_lint.rs index 9b646060adfd9..23457b8e0625c 100644 --- a/src/test/ui/feature-gates/feature-gate-non_exhaustive_omitted_patterns_lint.rs +++ b/src/test/ui/feature-gates/feature-gate-non_exhaustive_omitted_patterns_lint.rs @@ -3,9 +3,11 @@ #![deny(non_exhaustive_omitted_patterns)] //~^ WARNING unknown lint: `non_exhaustive_omitted_patterns` //~| WARNING unknown lint: `non_exhaustive_omitted_patterns` +//~| WARNING unknown lint: `non_exhaustive_omitted_patterns` #![allow(non_exhaustive_omitted_patterns)] //~^ WARNING unknown lint: `non_exhaustive_omitted_patterns` //~| WARNING unknown lint: `non_exhaustive_omitted_patterns` +//~| WARNING unknown lint: `non_exhaustive_omitted_patterns` fn main() { enum Foo { @@ -17,6 +19,8 @@ fn main() { //~| WARNING unknown lint: `non_exhaustive_omitted_patterns` //~| WARNING unknown lint: `non_exhaustive_omitted_patterns` //~| WARNING unknown lint: `non_exhaustive_omitted_patterns` + //~| WARNING unknown lint: `non_exhaustive_omitted_patterns` + //~| WARNING unknown lint: `non_exhaustive_omitted_patterns` match Foo::A { Foo::A => {} Foo::B => {} @@ -31,4 +35,5 @@ fn main() { } //~^^^ WARNING unknown lint: `non_exhaustive_omitted_patterns` //~| WARNING unknown lint: `non_exhaustive_omitted_patterns` + //~| WARNING unknown lint: `non_exhaustive_omitted_patterns` } diff --git a/src/test/ui/feature-gates/feature-gate-non_exhaustive_omitted_patterns_lint.stderr b/src/test/ui/feature-gates/feature-gate-non_exhaustive_omitted_patterns_lint.stderr index 3de08e215dada..29023858e4f3e 100644 --- a/src/test/ui/feature-gates/feature-gate-non_exhaustive_omitted_patterns_lint.stderr +++ b/src/test/ui/feature-gates/feature-gate-non_exhaustive_omitted_patterns_lint.stderr @@ -10,7 +10,7 @@ LL | #![deny(non_exhaustive_omitted_patterns)] = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable warning: unknown lint: `non_exhaustive_omitted_patterns` - --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:6:1 + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:7:1 | LL | #![allow(non_exhaustive_omitted_patterns)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -20,7 +20,7 @@ LL | #![allow(non_exhaustive_omitted_patterns)] = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable warning: unknown lint: `non_exhaustive_omitted_patterns` - --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:15:5 + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:17:5 | LL | #[allow(non_exhaustive_omitted_patterns)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -30,7 +30,7 @@ LL | #[allow(non_exhaustive_omitted_patterns)] = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable warning: unknown lint: `non_exhaustive_omitted_patterns` - --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:15:5 + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:17:5 | LL | #[allow(non_exhaustive_omitted_patterns)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -40,7 +40,7 @@ LL | #[allow(non_exhaustive_omitted_patterns)] = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable warning: unknown lint: `non_exhaustive_omitted_patterns` - --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:29:9 + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:33:9 | LL | #[warn(non_exhaustive_omitted_patterns)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -60,7 +60,7 @@ LL | #![deny(non_exhaustive_omitted_patterns)] = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable warning: unknown lint: `non_exhaustive_omitted_patterns` - --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:6:1 + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:7:1 | LL | #![allow(non_exhaustive_omitted_patterns)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -70,7 +70,7 @@ LL | #![allow(non_exhaustive_omitted_patterns)] = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable warning: unknown lint: `non_exhaustive_omitted_patterns` - --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:15:5 + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:17:5 | LL | #[allow(non_exhaustive_omitted_patterns)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -80,7 +80,7 @@ LL | #[allow(non_exhaustive_omitted_patterns)] = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable warning: unknown lint: `non_exhaustive_omitted_patterns` - --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:15:5 + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:17:5 | LL | #[allow(non_exhaustive_omitted_patterns)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -90,7 +90,7 @@ LL | #[allow(non_exhaustive_omitted_patterns)] = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable warning: unknown lint: `non_exhaustive_omitted_patterns` - --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:29:9 + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:33:9 | LL | #[warn(non_exhaustive_omitted_patterns)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -100,13 +100,13 @@ LL | #[warn(non_exhaustive_omitted_patterns)] = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable error[E0004]: non-exhaustive patterns: `Foo::C` not covered - --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:20:11 + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:24:11 | LL | match Foo::A { | ^^^^^^ pattern `Foo::C` not covered | note: `Foo` defined here - --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:12:15 + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:14:15 | LL | enum Foo { | --- @@ -119,6 +119,56 @@ LL ~ Foo::B => {} LL + Foo::C => todo!() | -error: aborting due to previous error; 10 warnings emitted +warning: unknown lint: `non_exhaustive_omitted_patterns` + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:3:1 + | +LL | #![deny(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `non_exhaustive_omitted_patterns` lint is unstable + = note: see issue #89554 for more information + = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable + +warning: unknown lint: `non_exhaustive_omitted_patterns` + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:7:1 + | +LL | #![allow(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `non_exhaustive_omitted_patterns` lint is unstable + = note: see issue #89554 for more information + = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable + +warning: unknown lint: `non_exhaustive_omitted_patterns` + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:17:5 + | +LL | #[allow(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `non_exhaustive_omitted_patterns` lint is unstable + = note: see issue #89554 for more information + = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable + +warning: unknown lint: `non_exhaustive_omitted_patterns` + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:17:5 + | +LL | #[allow(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `non_exhaustive_omitted_patterns` lint is unstable + = note: see issue #89554 for more information + = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable + +warning: unknown lint: `non_exhaustive_omitted_patterns` + --> $DIR/feature-gate-non_exhaustive_omitted_patterns_lint.rs:33:9 + | +LL | #[warn(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `non_exhaustive_omitted_patterns` lint is unstable + = note: see issue #89554 for more information + = help: add `#![feature(non_exhaustive_omitted_patterns_lint)]` to the crate attributes to enable + +error: aborting due to previous error; 15 warnings emitted For more information about this error, try `rustc --explain E0004`. From fca0d8a10e1a74a9a978c816929856a7e196eda2 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 10 Sep 2022 10:14:51 +0200 Subject: [PATCH 6/8] Comment LintLevelSets. --- compiler/rustc_lint/src/levels.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index eef342d4b07a2..b5b9312df63b4 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -28,8 +28,12 @@ use crate::errors::{ UnknownToolInScopedLint, }; +/// Collection of lint levels for the whole crate. +/// This is used by AST-based lints, which do not +/// wait until we have built HIR to be emitted. #[derive(Debug)] struct LintLevelSets { + /// Linked list of specifications. list: IndexVec, } @@ -40,12 +44,19 @@ rustc_index::newtype_index! { } } +/// Specifications found at this position in the stack. This map only represents the lints +/// found for one set of attributes (like `shallow_lint_levels_on` does). +/// +/// We store the level specifications as a linked list. +/// Each `LintSet` represents a set of attributes on the same AST node. +/// The `parent` forms a linked list that matches the AST tree. +/// This way, walking the linked list is equivalent to walking the AST bottom-up +/// to find the specifications for a given lint. #[derive(Debug)] struct LintSet { // -A,-W,-D flags, a `Symbol` for the flag itself and `Level` for which // flag. specs: FxHashMap, - parent: LintStackIndex, } From 1fcc4403916d13e12eba755b1234b8e6114ee9e3 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 10 Sep 2022 10:16:27 +0200 Subject: [PATCH 7/8] Add FIXME. --- compiler/rustc_lint/src/levels.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index b5b9312df63b4..a9ba2ad1d101d 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -908,6 +908,8 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { /// Checks if the lint is gated on a feature that is not enabled. /// /// Returns `true` if the lint's feature is enabled. + // FIXME only emit this once for each attribute, instead of repeating it 4 times for + // pre-expansion lints, post-expansion lints, `shallow_lint_levels_on` and `lint_expectations`. fn check_gated_lint(&self, lint_id: LintId, span: Span) -> bool { if let Some(feature) = lint_id.lint.feature_gate { if !self.sess.features_untracked().enabled(feature) { From 42a92eb54bd94985966eaaf0b7377679a78de835 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 10 Sep 2022 11:55:10 +0200 Subject: [PATCH 8/8] Correct Key impl for HirId. --- compiler/rustc_query_impl/src/keys.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_query_impl/src/keys.rs b/compiler/rustc_query_impl/src/keys.rs index ea2d10cd14bf7..31de83ee1418d 100644 --- a/compiler/rustc_query_impl/src/keys.rs +++ b/compiler/rustc_query_impl/src/keys.rs @@ -552,11 +552,11 @@ impl Key for HirId { } fn default_span(&self, tcx: TyCtxt<'_>) -> Span { - self.owner.default_span(tcx) + tcx.hir().span(*self) } #[inline(always)] fn key_as_def_id(&self) -> Option { - Some(self.owner.to_def_id()) + None } }