Auto merge of #116092 - compiler-errors:features-query, r=cjgillot

Unify `defined_lib_features` and `lib_features` queries

Extracts part of https://github.com/rust-lang/rust/pull/115623#discussion_r1318929750

I went with also introducing a `FeatureStability` enum, instead of using `Some(span)` to mean stable and `None` to mean unstable.

r? `@cjgillot`

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -21,6 +21,7 @@ use rustc_index::{Idx, IndexVec};
 use rustc_middle::metadata::ModChild;
 use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
 use rustc_middle::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo};
+use rustc_middle::middle::lib_features::LibFeatures;
 use rustc_middle::mir::interpret::{AllocDecodingSession, AllocDecodingState};
 use rustc_middle::ty::codec::TyDecoder;
 use rustc_middle::ty::fast_reject::SimplifiedType;
@@ -828,7 +829,7 @@ impl MetadataBlob {
                             out,
                             "{}{}",
                             feature,
-                            if let Some(since) = since {
+                            if let FeatureStability::AcceptedSince(since) = since {
                                 format!(" since {since}")
                             } else {
                                 String::new()
@@ -1176,8 +1177,15 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
     }
 
     /// Iterates over all the stability attributes in the given crate.
-    fn get_lib_features(self, tcx: TyCtxt<'tcx>) -> &'tcx [(Symbol, Option<Symbol>)] {
+    fn get_lib_features(self) -> LibFeatures {
-        tcx.arena.alloc_from_iter(self.root.lib_features.decode(self))
+        LibFeatures {
+            stability: self
+                .root
+                .lib_features
+                .decode(self)
+                .map(|(sym, stab)| (sym, (stab, DUMMY_SP)))
+                .collect(),
+        }
     }
 
     /// Iterates over the stability implications in the given crate (when a `#[unstable]` attribute

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -346,7 +346,7 @@ provide! { tcx, def_id, other, cdata,
     module_children => {
         tcx.arena.alloc_from_iter(cdata.get_module_children(def_id.index, tcx.sess))
     }
-    defined_lib_features => { cdata.get_lib_features(tcx) }
+    lib_features => { cdata.get_lib_features() }
     stability_implications => {
         cdata.get_stability_implications(tcx).iter().copied().collect()
     }

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -25,6 +25,7 @@ use rustc_middle::middle::dependency_format::Linkage;
 use rustc_middle::middle::exported_symbols::{
     metadata_symbol_name, ExportedSymbol, SymbolExportInfo,
 };
+use rustc_middle::middle::lib_features::FeatureStability;
 use rustc_middle::mir::interpret;
 use rustc_middle::query::LocalCrate;
 use rustc_middle::query::Providers;
@@ -1902,10 +1903,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
         self.lazy_array(deps.iter().map(|(_, dep)| dep))
     }
 
-    fn encode_lib_features(&mut self) -> LazyArray<(Symbol, Option<Symbol>)> {
+    fn encode_lib_features(&mut self) -> LazyArray<(Symbol, FeatureStability)> {
         empty_proc_macro!(self);
         let tcx = self.tcx;
-        let lib_features = tcx.lib_features(());
+        let lib_features = tcx.lib_features(LOCAL_CRATE);
         self.lazy_array(lib_features.to_vec())
     }
 

compiler/rustc_metadata/src/rmeta/mod.rs

@@ -3,6 +3,7 @@ use decoder::Metadata;
 use def_path_hash_map::DefPathHashMapRef;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
+use rustc_middle::middle::lib_features::FeatureStability;
 use table::TableBuilder;
 
 use rustc_ast as ast;
@@ -263,7 +264,7 @@ pub(crate) struct CrateRoot {
 
     crate_deps: LazyArray<CrateDep>,
     dylib_dependency_formats: LazyArray<Option<LinkagePreference>>,
-    lib_features: LazyArray<(Symbol, Option<Symbol>)>,
+    lib_features: LazyArray<(Symbol, FeatureStability)>,
     stability_implications: LazyArray<(Symbol, Symbol)>,
     lang_items: LazyArray<(DefIndex, LangItem)>,
     lang_items_missing: LazyArray<LangItem>,

compiler/rustc_middle/src/middle/mod.rs

@@ -7,22 +7,23 @@ pub mod lib_features {
     use rustc_data_structures::fx::FxHashMap;
     use rustc_span::{symbol::Symbol, Span};
 
-    #[derive(HashStable, Debug)]
+    #[derive(Copy, Clone, Debug, PartialEq, Eq)]
+    #[derive(HashStable, TyEncodable, TyDecodable)]
+    pub enum FeatureStability {
+        AcceptedSince(Symbol),
+        Unstable,
+    }
+
+    #[derive(HashStable, Debug, Default)]
     pub struct LibFeatures {
-        /// A map from feature to stabilisation version.
+        pub stability: FxHashMap<Symbol, (FeatureStability, Span)>,
-        pub stable: FxHashMap<Symbol, (Symbol, Span)>,
-        pub unstable: FxHashMap<Symbol, Span>,
     }
 
     impl LibFeatures {
-        pub fn to_vec(&self) -> Vec<(Symbol, Option<Symbol>)> {
+        pub fn to_vec(&self) -> Vec<(Symbol, FeatureStability)> {
-            let mut all_features: Vec<_> = self
+            let mut all_features: Vec<_> =
-                .stable
+                self.stability.iter().map(|(&sym, &(stab, _))| (sym, stab)).collect();
-                .iter()
+            all_features.sort_unstable_by(|(a, _), (b, _)| a.as_str().cmp(b.as_str()));
-                .map(|(f, (s, _))| (*f, Some(*s)))
-                .chain(self.unstable.keys().map(|f| (*f, None)))
-                .collect();
-            all_features.sort_unstable_by(|a, b| a.0.as_str().partial_cmp(b.0.as_str()).unwrap());
             all_features
         }
     }

compiler/rustc_middle/src/query/mod.rs

@@ -1732,13 +1732,10 @@ rustc_queries! {
         desc { |tcx| "computing crate imported by `{}`", tcx.def_path_str(def_id) }
     }
 
-    query lib_features(_: ()) -> &'tcx LibFeatures {
+    query lib_features(_: CrateNum) -> &'tcx LibFeatures {
-        arena_cache
-        desc { "calculating the lib features map" }
-    }
-    query defined_lib_features(_: CrateNum) -> &'tcx [(Symbol, Option<Symbol>)] {
         desc { "calculating the lib features defined in a crate" }
         separate_provide_extern
+        arena_cache
     }
     query stability_implications(_: CrateNum) -> &'tcx FxHashMap<Symbol, Symbol> {
         arena_cache

compiler/rustc_middle/src/ty/parameterized.rs

@@ -59,6 +59,7 @@ trivially_parameterized_over_tcx! {
     crate::middle::codegen_fn_attrs::CodegenFnAttrs,
     crate::middle::debugger_visualizer::DebuggerVisualizerFile,
     crate::middle::exported_symbols::SymbolExportInfo,
+    crate::middle::lib_features::FeatureStability,
     crate::middle::resolve_bound_vars::ObjectLifetimeDefault,
     crate::mir::ConstQualifs,
     ty::AssocItemContainer,

compiler/rustc_passes/src/lib_features.rs

@@ -8,18 +8,14 @@ use rustc_ast::Attribute;
 use rustc_attr::VERSION_PLACEHOLDER;
 use rustc_hir::intravisit::Visitor;
 use rustc_middle::hir::nested_filter;
-use rustc_middle::middle::lib_features::LibFeatures;
+use rustc_middle::middle::lib_features::{FeatureStability, LibFeatures};
-use rustc_middle::query::Providers;
+use rustc_middle::query::{LocalCrate, Providers};
 use rustc_middle::ty::TyCtxt;
 use rustc_span::symbol::Symbol;
 use rustc_span::{sym, Span};
 
 use crate::errors::{FeaturePreviouslyDeclared, FeatureStableTwice};
 
-fn new_lib_features() -> LibFeatures {
-    LibFeatures { stable: Default::default(), unstable: Default::default() }
-}
-
 pub struct LibFeatureCollector<'tcx> {
     tcx: TyCtxt<'tcx>,
     lib_features: LibFeatures,
@@ -27,10 +23,10 @@ pub struct LibFeatureCollector<'tcx> {
 
 impl<'tcx> LibFeatureCollector<'tcx> {
     fn new(tcx: TyCtxt<'tcx>) -> LibFeatureCollector<'tcx> {
-        LibFeatureCollector { tcx, lib_features: new_lib_features() }
+        LibFeatureCollector { tcx, lib_features: LibFeatures::default() }
     }
 
-    fn extract(&self, attr: &Attribute) -> Option<(Symbol, Option<Symbol>, Span)> {
+    fn extract(&self, attr: &Attribute) -> Option<(Symbol, FeatureStability, Span)> {
         let stab_attrs = [
             sym::stable,
             sym::unstable,
@@ -72,8 +68,11 @@ impl<'tcx> LibFeatureCollector<'tcx> {
                             | sym::rustc_const_unstable
                             | sym::rustc_default_body_unstable
                     );
-                    if since.is_some() || is_unstable {
+                    if is_unstable {
-                        return Some((feature, since, attr.span));
+                        return Some((feature, FeatureStability::Unstable, attr.span));
+                    }
+                    if let Some(since) = since {
+                        return Some((feature, FeatureStability::AcceptedSince(since), attr.span));
                     }
                 }
                 // We need to iterate over the other attributes, because
@@ -86,39 +85,39 @@ impl<'tcx> LibFeatureCollector<'tcx> {
         None
     }
 
-    fn collect_feature(&mut self, feature: Symbol, since: Option<Symbol>, span: Span) {
+    fn collect_feature(&mut self, feature: Symbol, stability: FeatureStability, span: Span) {
-        let already_in_stable = self.lib_features.stable.contains_key(&feature);
+        let existing_stability = self.lib_features.stability.get(&feature).cloned();
-        let already_in_unstable = self.lib_features.unstable.contains_key(&feature);
 
-        match (since, already_in_stable, already_in_unstable) {
+        match (stability, existing_stability) {
-            (Some(since), _, false) => {
+            (_, None) => {
-                if let Some((prev_since, _)) = self.lib_features.stable.get(&feature) {
+                self.lib_features.stability.insert(feature, (stability, span));
-                    if *prev_since != since {
+            }
-                        self.tcx.sess.emit_err(FeatureStableTwice {
+            (
-                            span,
+                FeatureStability::AcceptedSince(since),
-                            feature,
+                Some((FeatureStability::AcceptedSince(prev_since), _)),
-                            since,
+            ) => {
-                            prev_since: *prev_since,
+                if prev_since != since {
-                        });
+                    self.tcx.sess.emit_err(FeatureStableTwice { span, feature, since, prev_since });
-                        return;
-                    }
                 }
-
-                self.lib_features.stable.insert(feature, (since, span));
             }
-            (None, false, _) => {
+            (FeatureStability::AcceptedSince(_), Some((FeatureStability::Unstable, _))) => {
-                self.lib_features.unstable.insert(feature, span);
-            }
-            (Some(_), _, true) | (None, true, _) => {
-                let declared = if since.is_some() { "stable" } else { "unstable" };
-                let prev_declared = if since.is_none() { "stable" } else { "unstable" };
                 self.tcx.sess.emit_err(FeaturePreviouslyDeclared {
                     span,
                     feature,
-                    declared,
+                    declared: "stable",
-                    prev_declared,
+                    prev_declared: "unstable",
                 });
             }
+            (FeatureStability::Unstable, Some((FeatureStability::AcceptedSince(_), _))) => {
+                self.tcx.sess.emit_err(FeaturePreviouslyDeclared {
+                    span,
+                    feature,
+                    declared: "unstable",
+                    prev_declared: "stable",
+                });
+            }
+            // duplicate `unstable` feature is ok.
+            (FeatureStability::Unstable, Some((FeatureStability::Unstable, _))) => {}
         }
     }
 }
@@ -137,11 +136,11 @@ impl<'tcx> Visitor<'tcx> for LibFeatureCollector<'tcx> {
     }
 }
 
-fn lib_features(tcx: TyCtxt<'_>, (): ()) -> LibFeatures {
+fn lib_features(tcx: TyCtxt<'_>, LocalCrate: LocalCrate) -> LibFeatures {
     // If `staged_api` is not enabled then we aren't allowed to define lib
     // features; there is no point collecting them.
     if !tcx.features().staged_api {
-        return new_lib_features();
+        return LibFeatures::default();
     }
 
     let mut collector = LibFeatureCollector::new(tcx);

compiler/rustc_passes/src/stability.rs

@@ -9,11 +9,12 @@ use rustc_attr::{
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::def_id::{LocalDefId, LocalModDefId, CRATE_DEF_ID};
+use rustc_hir::def_id::{LocalDefId, LocalModDefId, CRATE_DEF_ID, LOCAL_CRATE};
 use rustc_hir::hir_id::CRATE_HIR_ID;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::{FieldDef, Item, ItemKind, TraitRef, Ty, TyKind, Variant};
 use rustc_middle::hir::nested_filter;
+use rustc_middle::middle::lib_features::{FeatureStability, LibFeatures};
 use rustc_middle::middle::privacy::EffectiveVisibilities;
 use rustc_middle::middle::stability::{AllowUnstable, DeprecationEntry, Index};
 use rustc_middle::query::Providers;
@@ -978,29 +979,27 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
         tcx: TyCtxt<'tcx>,
         remaining_lib_features: &mut FxIndexMap<&Symbol, Span>,
         remaining_implications: &mut FxHashMap<Symbol, Symbol>,
-        defined_features: &[(Symbol, Option<Symbol>)],
+        defined_features: &LibFeatures,
         all_implications: &FxHashMap<Symbol, Symbol>,
     ) {
-        for (feature, since) in defined_features {
+        for (feature, since) in defined_features.to_vec() {
-            if let Some(since) = since
+            if let FeatureStability::AcceptedSince(since) = since
                 && let Some(span) = remaining_lib_features.get(&feature)
             {
                 // Warn if the user has enabled an already-stable lib feature.
                 if let Some(implies) = all_implications.get(&feature) {
-                    unnecessary_partially_stable_feature_lint(
+                    unnecessary_partially_stable_feature_lint(tcx, *span, feature, *implies, since);
-                        tcx, *span, *feature, *implies, *since,
-                    );
                 } else {
-                    unnecessary_stable_feature_lint(tcx, *span, *feature, *since);
+                    unnecessary_stable_feature_lint(tcx, *span, feature, since);
                 }
             }
-            remaining_lib_features.remove(feature);
+            remaining_lib_features.remove(&feature);
 
             // `feature` is the feature doing the implying, but `implied_by` is the feature with
             // the attribute that establishes this relationship. `implied_by` is guaranteed to be a
             // feature defined in the local crate because `remaining_implications` is only the
             // implications from this crate.
-            remaining_implications.remove(feature);
+            remaining_implications.remove(&feature);
 
             if remaining_lib_features.is_empty() && remaining_implications.is_empty() {
                 break;
@@ -1009,12 +1008,11 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
     }
 
     // All local crate implications need to have the feature that implies it confirmed to exist.
-    let mut remaining_implications =
+    let mut remaining_implications = tcx.stability_implications(LOCAL_CRATE).clone();
-        tcx.stability_implications(rustc_hir::def_id::LOCAL_CRATE).clone();
 
     // We always collect the lib features declared in the current crate, even if there are
     // no unknown features, because the collection also does feature attribute validation.
-    let local_defined_features = tcx.lib_features(()).to_vec();
+    let local_defined_features = tcx.lib_features(LOCAL_CRATE);
     if !remaining_lib_features.is_empty() || !remaining_implications.is_empty() {
         // Loading the implications of all crates is unavoidable to be able to emit the partial
         // stabilization diagnostic, but it can be avoided when there are no
@@ -1028,7 +1026,7 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
             tcx,
             &mut remaining_lib_features,
             &mut remaining_implications,
-            local_defined_features.as_slice(),
+            local_defined_features,
             &all_implications,
         );
 
@@ -1040,7 +1038,7 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
                 tcx,
                 &mut remaining_lib_features,
                 &mut remaining_implications,
-                tcx.defined_lib_features(cnum).to_vec().as_slice(),
+                tcx.lib_features(cnum),
                 &all_implications,
             );
         }
@@ -1051,13 +1049,12 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
     }
 
     for (implied_by, feature) in remaining_implications {
-        let local_defined_features = tcx.lib_features(());
+        let local_defined_features = tcx.lib_features(LOCAL_CRATE);
-        let span = *local_defined_features
+        let span = local_defined_features
-            .stable
+            .stability
             .get(&feature)
-            .map(|(_, span)| span)
+            .expect("feature that implied another does not exist")
-            .or_else(|| local_defined_features.unstable.get(&feature))
+            .1;
-            .expect("feature that implied another does not exist");
         tcx.sess.emit_err(errors::ImpliedFeatureNotExist { span, feature, implied_by });
     }