diff --git a/Cargo.lock b/Cargo.lock index bc2c77d9271..8ca6f26e326 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3882,7 +3882,7 @@ version = "0.0.0" dependencies = [ "rustc_ast", "rustc_data_structures", - "rustc_index", + "rustc_hir", "rustc_macros", "rustc_serialize", "rustc_span", diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 83e52e002ae..0fd057f434b 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -25,7 +25,7 @@ use Level::*; use emitter::{is_case_difference, Emitter, EmitterWriter}; use registry::Registry; -use rustc_data_structures::fx::{FxHashSet, FxIndexMap}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; use rustc_data_structures::stable_hasher::StableHasher; use rustc_data_structures::sync::{self, Lock, Lrc}; use rustc_data_structures::AtomicRef; @@ -452,7 +452,14 @@ struct HandlerInner { future_breakage_diagnostics: Vec, - /// Lint [`Diagnostic`]s can be expected as described in [RFC-2383]. An + /// Expected [`Diagnostic`]s store a [`LintExpectationId`] as part of + /// the lint level. [`LintExpectationId`]s created early during the compilation + /// (before `HirId`s have been defined) are not stable and can therefore not be + /// stored on disk. This buffer stores these diagnostics until the ID has been + /// replaced by a stable [`LintExpectationId`]. The [`Diagnostic`]s are the + /// submitted for storage and added to the list of fulfilled expectations. + unstable_expect_diagnostics: Vec, + /// expected diagnostic will have the level `Expect` which additionally /// carries the [`LintExpectationId`] of the expectation that can be /// marked as fulfilled. This is a collection of all [`LintExpectationId`]s @@ -580,6 +587,7 @@ impl Handler { emitted_diagnostics: Default::default(), stashed_diagnostics: Default::default(), future_breakage_diagnostics: Vec::new(), + unstable_expect_diagnostics: Vec::new(), fulfilled_expectations: Default::default(), }), } @@ -923,6 +931,28 @@ impl Handler { self.inner.borrow_mut().emit_unused_externs(lint_level, unused_externs) } + pub fn update_unstable_expectation_id( + &self, + unstable_to_stable: &FxHashMap, + ) { + let diags = std::mem::take(&mut self.inner.borrow_mut().unstable_expect_diagnostics); + if diags.is_empty() { + return; + } + + let mut inner = self.inner.borrow_mut(); + for mut diag in diags.into_iter() { + if let Some(unstable_id) = diag.level.get_expectation_id() { + if let Some(stable_id) = unstable_to_stable.get(&unstable_id) { + diag.level = Level::Expect(*stable_id); + inner.fulfilled_expectations.insert(*stable_id); + } + } + + (*TRACK_DIAGNOSTICS)(&diag); + } + } + /// This methods steals all [`LintExpectationId`]s that are stored inside /// [`HandlerInner`] and indicate that the linked expectation has been fulfilled. pub fn steal_fulfilled_expectation_ids(&self) -> FxHashSet { @@ -973,6 +1003,15 @@ impl HandlerInner { return; } + // The `LintExpectationId` can be stable or unstable depending on when it was created. + // Diagnostics created before the definition of `HirId`s are unstable and can not yet + // be stored. Instead, they are buffered until the `LintExpectationId` is replaced by + // a stable one by the `LintLevelsBuilder`. + if let Level::Expect(LintExpectationId::Unstable(_)) = diagnostic.level { + self.unstable_expect_diagnostics.push(diagnostic.clone()); + return; + } + (*TRACK_DIAGNOSTICS)(diagnostic); if let Level::Expect(expectation_id) = diagnostic.level { @@ -1322,6 +1361,13 @@ impl Level { pub fn is_failure_note(&self) -> bool { matches!(*self, FailureNote) } + + pub fn get_expectation_id(&self) -> Option { + match self { + Level::Expect(id) => Some(*id), + _ => None, + } + } } // FIXME(eddyb) this doesn't belong here AFAICT, should be moved to callsite. diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index 1b2c88867d4..e9b7620bf1d 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -59,7 +59,8 @@ impl<'a, T: EarlyLintPass> EarlyContextAndPass<'a, T> { F: FnOnce(&mut Self), { let is_crate_node = id == ast::CRATE_NODE_ID; - let push = self.context.builder.push(attrs, is_crate_node); + let push = self.context.builder.push(attrs, is_crate_node, None); + self.check_id(id); self.enter_attrs(attrs); f(self); diff --git a/compiler/rustc_lint/src/expect.rs b/compiler/rustc_lint/src/expect.rs index a0c8b3501cd..49664322a98 100644 --- a/compiler/rustc_lint/src/expect.rs +++ b/compiler/rustc_lint/src/expect.rs @@ -25,7 +25,7 @@ pub fn check_expectations(tcx: TyCtxt<'_>) { } fn emit_unfulfilled_expectation_lint(tcx: TyCtxt<'_>, expectation: &LintExpectation) { - // FIXME The current implementation doesn't cover cases where the + // FIXME: The current implementation doesn't cover cases where the // `unfulfilled_lint_expectations` is actually expected by another lint // expectation. This can be added here as we have the lint level of this // expectation, and we can also mark the lint expectation it would fulfill diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index a876e35c0a8..417022bd2bf 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -32,17 +32,23 @@ fn lint_levels(tcx: TyCtxt<'_>, (): ()) -> LintLevelMap { builder.levels.id_to_set.reserve(krate.owners.len() + 1); - let push = builder.levels.push(tcx.hir().attrs(hir::CRATE_HIR_ID), true); + let push = + builder.levels.push(tcx.hir().attrs(hir::CRATE_HIR_ID), true, Some(hir::CRATE_HIR_ID)); + builder.levels.register_id(hir::CRATE_HIR_ID); tcx.hir().walk_toplevel_module(&mut builder); builder.levels.pop(push); + builder.levels.update_unstable_expectation_ids(); builder.levels.build_map() } pub struct LintLevelsBuilder<'s> { sess: &'s Session, lint_expectations: FxHashMap, + /// 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, sets: LintLevelSets, id_to_set: FxHashMap, cur: LintStackIndex, @@ -66,6 +72,7 @@ impl<'s> LintLevelsBuilder<'s> { 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(), @@ -226,13 +233,26 @@ impl<'s> LintLevelsBuilder<'s> { /// `#[allow]` /// /// Don't forget to call `pop`! - pub(crate) fn push(&mut self, attrs: &[ast::Attribute], is_crate_node: bool) -> BuilderPush { + pub(crate) fn push( + &mut self, + attrs: &[ast::Attribute], + is_crate_node: bool, + source_hir_id: Option, + ) -> BuilderPush { let mut specs = FxHashMap::default(); let sess = self.sess; let bad_attr = |span| struct_span_err!(sess, span, E0452, "malformed lint attribute input"); - for attr in attrs { - let Some(level) = Level::from_symbol(attr.name_or_empty(), attr.id.as_u32()) else { - continue + for (attr_index, attr) in attrs.iter().enumerate() { + let level = match Level::from_symbol(attr.name_or_empty(), || { + LintExpectationId::Unstable(attr.id) + }) { + None => continue, + Some(Level::Expect(unstable_id)) if source_hir_id.is_some() => { + let stable_id = + self.create_stable_id(unstable_id, source_hir_id.unwrap(), attr_index); + Level::Expect(stable_id) + } + Some(lvl) => lvl, }; let Some(mut metas) = attr.meta_item_list() else { @@ -539,6 +559,19 @@ impl<'s> LintLevelsBuilder<'s> { 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 }; + + self.expectation_id_map.insert(unstable_id, stable_id); + + stable_id + } + /// Checks if the lint is gated on a feature that is not enabled. fn check_gated_lint(&self, lint_id: LintId, span: Span) { if let Some(feature) = lint_id.lint.feature_gate { @@ -582,6 +615,10 @@ impl<'s> LintLevelsBuilder<'s> { 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, @@ -603,7 +640,8 @@ impl LintLevelMapBuilder<'_> { { 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); + let push = self.levels.push(attrs, is_crate_hir, Some(id)); + if push.changed { self.levels.register_id(id); } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 2dc6e980722..18f229564c2 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -51,8 +51,8 @@ pub mod builtin; mod context; mod early; mod enum_intrinsics_non_enums; -pub mod hidden_unicode_codepoints; mod expect; +pub mod hidden_unicode_codepoints; mod internal; mod late; mod levels; diff --git a/compiler/rustc_lint_defs/Cargo.toml b/compiler/rustc_lint_defs/Cargo.toml index c37e45b46ce..8acf7943de9 100644 --- a/compiler/rustc_lint_defs/Cargo.toml +++ b/compiler/rustc_lint_defs/Cargo.toml @@ -10,4 +10,4 @@ rustc_span = { path = "../rustc_span" } rustc_serialize = { path = "../rustc_serialize" } rustc_macros = { path = "../rustc_macros" } rustc_target = { path = "../rustc_target" } -rustc_index = { path = "../rustc_index" } +rustc_hir = { path = "../rustc_hir" } diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 6fca39b2c6b..5e3b6889fc6 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -5,7 +5,9 @@ extern crate rustc_macros; pub use self::Level::*; use rustc_ast::node_id::{NodeId, NodeMap}; +use rustc_ast::AttrId; use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey}; +use rustc_hir::HirId; use rustc_serialize::json::Json; use rustc_span::edition::Edition; use rustc_span::{sym, symbol::Ident, MultiSpan, Span, Symbol}; @@ -48,29 +50,70 @@ pub enum Applicability { Unspecified, } -rustc_index::newtype_index! { - /// FIXME: The lint expectation ID is currently a simple copy of the `AttrId` - /// that the expectation originated from. In the future it should be generated - /// by other means. This is for one to keep the IDs independent of each other - /// and also to ensure that it is actually stable between compilation sessions. - /// (The `AttrId` for instance, is not stable). - /// - /// Additionally, it would be nice if this generation could be moved into - /// [`Level::from_symbol`] to have it all contained in one module and to - /// make it simpler to use. - pub struct LintExpectationId { - DEBUG_FORMAT = "LintExpectationId({})" +/// Each lint expectation has a `LintExpectationId` assigned by the +/// [`LintLevelsBuilder`][`rustc_lint::levels::LintLevelsBuilder`]. Expected +/// [`Diagnostic`][`rustc_errors::Diagnostic`]s get the lint level `Expect` which +/// stores the `LintExpectationId` to match it with the actual expectation later on. +/// +/// The `LintExpectationId` has to be has stable between compilations, as diagnostic +/// instances might be loaded from cache. Lint messages can be emitted during an +/// `EarlyLintPass` operating on the AST and during a `LateLintPass` traversing the +/// HIR tree. The AST doesn't have enough information to create a stable id. The +/// `LintExpectationId` will instead store the [`AttrId`] defining the expectation. +/// These `LintExpectationId` will be updated to use the stable [`HirId`] once the +/// AST has been lowered. The transformation is done by the +/// [`LintLevelsBuilder`][`rustc_lint::levels::LintLevelsBuilder`] +#[derive(Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash, Encodable, Decodable)] +pub enum LintExpectationId { + /// Used for lints emitted during the `EarlyLintPass`. This id is not + /// has stable and should not be cached. + Unstable(AttrId), + /// The [`HirId`] that the lint expectation is attached to. This id is + /// 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: usize }, +} + +impl LintExpectationId { + pub fn is_stable(&self) -> bool { + match self { + LintExpectationId::Unstable(_) => false, + LintExpectationId::Stable { .. } => true, + } } } -rustc_data_structures::impl_stable_hash_via_hash!(LintExpectationId); +impl HashStable for LintExpectationId { + #[inline] + fn hash_stable(&self, hcx: &mut HCX, hasher: &mut StableHasher) { + match self { + LintExpectationId::Unstable(_) => { + unreachable!( + "HashStable should never be called for an unstable `LintExpectationId`" + ) + } + LintExpectationId::Stable { hir_id, attr_index } => { + hir_id.hash_stable(hcx, hasher); + attr_index.hash_stable(hcx, hasher); + } + } + } +} -impl ToStableHashKey for LintExpectationId { - type KeyType = u32; +impl ToStableHashKey for LintExpectationId { + type KeyType = (HirId, usize); #[inline] fn to_stable_hash_key(&self, _: &HCX) -> Self::KeyType { - self.as_u32() + match self { + LintExpectationId::Unstable(_) => { + unreachable!( + "HashStable should never be called for an unstable `LintExpectationId`" + ) + } + LintExpectationId::Stable { hir_id, attr_index } => (*hir_id, *attr_index), + } } } @@ -133,10 +176,13 @@ impl Level { } /// Converts a symbol to a level. - pub fn from_symbol(x: Symbol, possible_lint_expect_id: u32) -> Option { + pub fn from_symbol(x: Symbol, create_expectation_id: F) -> Option + where + F: FnOnce() -> LintExpectationId, + { match x { sym::allow => Some(Level::Allow), - sym::expect => Some(Level::Expect(LintExpectationId::from(possible_lint_expect_id))), + sym::expect => Some(Level::Expect(create_expectation_id())), sym::warn => Some(Level::Warn), sym::deny => Some(Level::Deny), sym::forbid => Some(Level::Forbid), diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index 9eb7aca13c0..f680843f9e4 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -6,10 +6,9 @@ use rustc_errors::{Diagnostic, DiagnosticBuilder, DiagnosticId}; use rustc_hir::HirId; use rustc_index::vec::IndexVec; use rustc_query_system::ich::StableHashingContext; -use rustc_session::lint::LintExpectationId; use rustc_session::lint::{ builtin::{self, FORBIDDEN_LINT_GROUPS}, - FutureIncompatibilityReason, Level, Lint, LintId, + FutureIncompatibilityReason, Level, Lint, LintExpectationId, LintId, }; use rustc_session::{DiagnosticMessageId, Session}; use rustc_span::hygiene::MacroKind; diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index bd48a9867f9..5165193ab54 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -49,7 +49,7 @@ use rustc_middle::mir::FakeReadCause; use rustc_query_system::ich::{NodeIdHashingMode, StableHashingContext}; use rustc_serialize::opaque::{FileEncodeResult, FileEncoder}; use rustc_session::config::{BorrowckMode, CrateType, OutputFilenames}; -use rustc_session::lint::{Level, Lint}; +use rustc_session::lint::{Level, Lint, LintExpectationId}; use rustc_session::Limit; use rustc_session::Session; use rustc_span::def_id::{DefPathHash, StableCrateId}; @@ -2755,11 +2755,13 @@ impl<'tcx> TyCtxt<'tcx> { return bound; } - if hir - .attrs(id) - .iter() - .any(|attr| Level::from_symbol(attr.name_or_empty(), attr.id.as_u32()).is_some()) - { + if hir.attrs(id).iter().enumerate().any(|(attr_index, attr)| { + Level::from_symbol(attr.name_or_empty(), || LintExpectationId::Stable { + hir_id: id, + attr_index, + }) + .is_some() + }) { return id; } let next = hir.get_parent_node(id);