Auto merge of #143845 - cjgillot:stability-query, r=jieyouxu

Split-up stability_index query

This PR aims to move deprecation and stability processing away from the monolithic `stability_index` query, and directly implement `lookup_{deprecation,stability,body_stability,const_stability}` queries.

The basic idea is to:
- move per-attribute sanity checks into `check_attr.rs`;
- move attribute compatibility checks into the `MissingStabilityAnnotations` visitor;
- progressively dismantle the `Annotator` visitor and the `stability_index` query.

The first commit contains functional change, and now warns when `#[automatically_derived]` is applied on a non-trait impl block. The other commits should not change visible behaviour.

Perf in #143845 (comment) shows small but consistent improvement, except for unused-warnings case. That case being a stress test, I'm leaning towards accepting the regression.

This PR changes `check_attr`, so has a high conflict rate on that file. This should not cause issues for review.

Author: bors

compiler/rustc_attr_data_structures/src/lib.rs

@@ -24,7 +24,7 @@ use rustc_ast::token::CommentKind;
 use rustc_ast::{AttrStyle, IntTy, UintTy};
 use rustc_ast_pretty::pp::Printer;
 use rustc_span::hygiene::Transparency;
-use rustc_span::{Span, Symbol};
+use rustc_span::{ErrorGuaranteed, Span, Symbol};
 pub use stability::*;
 use thin_vec::ThinVec;
 pub use version::*;
@@ -170,7 +170,7 @@ macro_rules! print_tup {
 }
 
 print_tup!(A B C D E F G H);
-print_skip!(Span, ());
+print_skip!(Span, (), ErrorGuaranteed);
 print_disp!(u16, bool, NonZero<u32>);
 print_debug!(Symbol, UintTy, IntTy, Align, AttrStyle, CommentKind, Transparency);
 

compiler/rustc_attr_data_structures/src/stability.rs

@@ -1,7 +1,7 @@
 use std::num::NonZero;
 
 use rustc_macros::{Decodable, Encodable, HashStable_Generic, PrintAttribute};
-use rustc_span::{Symbol, sym};
+use rustc_span::{ErrorGuaranteed, Symbol, sym};
 
 use crate::{PrintAttribute, RustcVersion};
 
@@ -153,7 +153,7 @@ pub enum StableSince {
     /// Stabilized in the upcoming version, whatever number that is.
     Current,
     /// Failed to parse a stabilization version.
-    Err,
+    Err(ErrorGuaranteed),
 }
 
 impl StabilityLevel {

compiler/rustc_attr_parsing/src/attributes/stability.rs

@@ -292,12 +292,12 @@ pub(crate) fn parse_stability<S: Stage>(
         } else if let Some(version) = parse_version(since) {
             StableSince::Version(version)
         } else {
-            cx.emit_err(session_diagnostics::InvalidSince { span: cx.attr_span });
-            StableSince::Err
+            let err = cx.emit_err(session_diagnostics::InvalidSince { span: cx.attr_span });
+            StableSince::Err(err)
         }
     } else {
-        cx.emit_err(session_diagnostics::MissingSince { span: cx.attr_span });
-        StableSince::Err
+        let err = cx.emit_err(session_diagnostics::MissingSince { span: cx.attr_span });
+        StableSince::Err(err)
     };
 
     match feature {

compiler/rustc_hir/src/target.rs

@@ -41,7 +41,7 @@ pub enum Target {
     Union,
     Trait,
     TraitAlias,
-    Impl,
+    Impl { of_trait: bool },
     Expression,
     Statement,
     Arm,
@@ -51,7 +51,7 @@ pub enum Target {
     ForeignFn,
     ForeignStatic,
     ForeignTy,
-    GenericParam(GenericParamKind),
+    GenericParam { kind: GenericParamKind, has_default: bool },
     MacroDef,
     Param,
     PatField,
@@ -86,14 +86,14 @@ impl Target {
             | Target::Union
             | Target::Trait
             | Target::TraitAlias
-            | Target::Impl
+            | Target::Impl { .. }
             | Target::Expression
             | Target::Statement
             | Target::Arm
             | Target::ForeignFn
             | Target::ForeignStatic
             | Target::ForeignTy
-            | Target::GenericParam(_)
+            | Target::GenericParam { .. }
             | Target::MacroDef
             | Target::Param
             | Target::PatField
@@ -119,7 +119,7 @@ impl Target {
             ItemKind::Union(..) => Target::Union,
             ItemKind::Trait(..) => Target::Trait,
             ItemKind::TraitAlias(..) => Target::TraitAlias,
-            ItemKind::Impl { .. } => Target::Impl,
+            ItemKind::Impl(imp_) => Target::Impl { of_trait: imp_.of_trait.is_some() },
         }
     }
 
@@ -141,7 +141,7 @@ impl Target {
             DefKind::Union => Target::Union,
             DefKind::Trait => Target::Trait,
             DefKind::TraitAlias => Target::TraitAlias,
-            DefKind::Impl { .. } => Target::Impl,
+            DefKind::Impl { of_trait } => Target::Impl { of_trait },
             _ => panic!("impossible case reached"),
         }
     }
@@ -169,11 +169,17 @@ impl Target {
 
     pub fn from_generic_param(generic_param: &hir::GenericParam<'_>) -> Target {
         match generic_param.kind {
-            hir::GenericParamKind::Type { .. } => Target::GenericParam(GenericParamKind::Type),
+            hir::GenericParamKind::Type { default, .. } => Target::GenericParam {
+                kind: GenericParamKind::Type,
+                has_default: default.is_some(),
+            },
             hir::GenericParamKind::Lifetime { .. } => {
-                Target::GenericParam(GenericParamKind::Lifetime)
+                Target::GenericParam { kind: GenericParamKind::Lifetime, has_default: false }
             }
-            hir::GenericParamKind::Const { .. } => Target::GenericParam(GenericParamKind::Const),
+            hir::GenericParamKind::Const { default, .. } => Target::GenericParam {
+                kind: GenericParamKind::Const,
+                has_default: default.is_some(),
+            },
         }
     }
 
@@ -196,7 +202,8 @@ impl Target {
             Target::Union => "union",
             Target::Trait => "trait",
             Target::TraitAlias => "trait alias",
-            Target::Impl => "implementation block",
+            Target::Impl { of_trait: false } => "inherent implementation block",
+            Target::Impl { of_trait: true } => "trait implementation block",
             Target::Expression => "expression",
             Target::Statement => "statement",
             Target::Arm => "match arm",
@@ -210,7 +217,7 @@ impl Target {
             Target::ForeignFn => "foreign function",
             Target::ForeignStatic => "foreign static item",
             Target::ForeignTy => "foreign type",
-            Target::GenericParam(kind) => match kind {
+            Target::GenericParam { kind, has_default: _ } => match kind {
                 GenericParamKind::Type => "type parameter",
                 GenericParamKind::Lifetime => "lifetime parameter",
                 GenericParamKind::Const => "const parameter",

compiler/rustc_interface/src/passes.rs

@@ -1054,18 +1054,12 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
                     tcx.ensure_ok().check_mod_unstable_api_usage(module);
                 });
             },
-            {
-                sess.time("unused_lib_feature_checking", || {
-                    rustc_passes::stability::check_unused_or_stable_features(tcx)
-                });
-            },
             {
                 // We force these queries to run,
                 // since they might not otherwise get called.
                 // This marks the corresponding crate-level attributes
                 // as used, and ensures that their values are valid.
                 tcx.ensure_ok().limits(());
-                tcx.ensure_ok().stability_index(());
             }
         );
     });

compiler/rustc_middle/src/middle/stability.rs

@@ -7,10 +7,9 @@ use rustc_ast::NodeId;
 use rustc_attr_data_structures::{
     self as attrs, ConstStability, DefaultBodyStability, DeprecatedSince, Deprecation, Stability,
 };
-use rustc_data_structures::unord::UnordMap;
 use rustc_errors::{Applicability, Diag, EmissionGuarantee};
 use rustc_feature::GateIssue;
-use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdMap};
+use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::{self as hir, HirId};
 use rustc_macros::{Decodable, Encodable, HashStable, Subdiagnostic};
 use rustc_session::Session;
@@ -65,48 +64,6 @@ impl DeprecationEntry {
     }
 }
 
-/// A stability index, giving the stability level for items and methods.
-#[derive(HashStable, Debug)]
-pub struct Index {
-    /// This is mostly a cache, except the stabilities of local items
-    /// are filled by the annotator.
-    pub stab_map: LocalDefIdMap<Stability>,
-    pub const_stab_map: LocalDefIdMap<ConstStability>,
-    pub default_body_stab_map: LocalDefIdMap<DefaultBodyStability>,
-    pub depr_map: LocalDefIdMap<DeprecationEntry>,
-    /// Mapping from feature name to feature name based on the `implied_by` field of `#[unstable]`
-    /// attributes. If a `#[unstable(feature = "implier", implied_by = "impliee")]` attribute
-    /// exists, then this map will have a `impliee -> implier` entry.
-    ///
-    /// This mapping is necessary unless both the `#[stable]` and `#[unstable]` attributes should
-    /// specify their implications (both `implies` and `implied_by`). If only one of the two
-    /// attributes do (as in the current implementation, `implied_by` in `#[unstable]`), then this
-    /// mapping is necessary for diagnostics. When a "unnecessary feature attribute" error is
-    /// reported, only the `#[stable]` attribute information is available, so the map is necessary
-    /// to know that the feature implies another feature. If it were reversed, and the `#[stable]`
-    /// attribute had an `implies` meta item, then a map would be necessary when avoiding a "use of
-    /// unstable feature" error for a feature that was implied.
-    pub implications: UnordMap<Symbol, Symbol>,
-}
-
-impl Index {
-    pub fn local_stability(&self, def_id: LocalDefId) -> Option<Stability> {
-        self.stab_map.get(&def_id).copied()
-    }
-
-    pub fn local_const_stability(&self, def_id: LocalDefId) -> Option<ConstStability> {
-        self.const_stab_map.get(&def_id).copied()
-    }
-
-    pub fn local_default_body_stability(&self, def_id: LocalDefId) -> Option<DefaultBodyStability> {
-        self.default_body_stab_map.get(&def_id).copied()
-    }
-
-    pub fn local_deprecation_entry(&self, def_id: LocalDefId) -> Option<DeprecationEntry> {
-        self.depr_map.get(&def_id).cloned()
-    }
-}
-
 pub fn report_unstable(
     sess: &Session,
     feature: Symbol,

compiler/rustc_middle/src/query/mod.rs

@@ -112,7 +112,7 @@ use crate::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo};
 use crate::middle::lib_features::LibFeatures;
 use crate::middle::privacy::EffectiveVisibilities;
 use crate::middle::resolve_bound_vars::{ObjectLifetimeDefault, ResolveBoundVars, ResolvedArg};
-use crate::middle::stability::{self, DeprecationEntry};
+use crate::middle::stability::DeprecationEntry;
 use crate::mir::interpret::{
     EvalStaticInitializerRawResult, EvalToAllocationRawResult, EvalToConstValueResult,
     EvalToValTreeResult, GlobalId, LitToConstInput,
@@ -2171,6 +2171,18 @@ rustc_queries! {
         separate_provide_extern
         arena_cache
     }
+    /// Mapping from feature name to feature name based on the `implied_by` field of `#[unstable]`
+    /// attributes. If a `#[unstable(feature = "implier", implied_by = "impliee")]` attribute
+    /// exists, then this map will have a `impliee -> implier` entry.
+    ///
+    /// This mapping is necessary unless both the `#[stable]` and `#[unstable]` attributes should
+    /// specify their implications (both `implies` and `implied_by`). If only one of the two
+    /// attributes do (as in the current implementation, `implied_by` in `#[unstable]`), then this
+    /// mapping is necessary for diagnostics. When a "unnecessary feature attribute" error is
+    /// reported, only the `#[stable]` attribute information is available, so the map is necessary
+    /// to know that the feature implies another feature. If it were reversed, and the `#[stable]`
+    /// attribute had an `implies` meta item, then a map would be necessary when avoiding a "use of
+    /// unstable feature" error for a feature that was implied.
     query stability_implications(_: CrateNum) -> &'tcx UnordMap<Symbol, Symbol> {
         arena_cache
         desc { "calculating the implications between `#[unstable]` features defined in a crate" }
@@ -2277,11 +2289,6 @@ rustc_queries! {
         desc { "fetching potentially unused trait imports" }
     }
 
-    query stability_index(_: ()) -> &'tcx stability::Index {
-        arena_cache
-        eval_always
-        desc { "calculating the stability index for the local crate" }
-    }
     /// All available crates in the graph, including those that should not be user-facing
     /// (such as private crates).
     query crates(_: ()) -> &'tcx [CrateNum] {

compiler/rustc_middle/src/ty/context.rs

@@ -65,7 +65,7 @@ use crate::infer::canonical::{CanonicalParamEnvCache, CanonicalVarKind, Canonica
 use crate::lint::lint_level;
 use crate::metadata::ModChild;
 use crate::middle::codegen_fn_attrs::{CodegenFnAttrs, TargetFeature};
-use crate::middle::{resolve_bound_vars, stability};
+use crate::middle::resolve_bound_vars;
 use crate::mir::interpret::{self, Allocation, ConstAllocation};
 use crate::mir::{Body, Local, Place, PlaceElem, ProjectionKind, Promoted};
 use crate::query::plumbing::QuerySystem;
@@ -1807,10 +1807,6 @@ impl<'tcx> TyCtxt<'tcx> {
         )
     }
 
-    pub fn stability(self) -> &'tcx stability::Index {
-        self.stability_index(())
-    }
-
     pub fn features(self) -> &'tcx rustc_feature::Features {
         self.features_query(())
     }

compiler/rustc_passes/messages.ftl

@@ -560,7 +560,8 @@ passes_only_has_effect_on =
     `#[{$attr_name}]` only has an effect on {$target_name ->
         [function] functions
         [module] modules
-        [implementation_block] implementation blocks
+        [trait_implementation_block] trait implementation blocks
+        [inherent_implementation_block] inherent implementation blocks
         *[unspecified] (unspecified--this is a compiler bug)
     }