Auto merge of #100328 - davidtwco:perf-implications, r=nnethercote

passes: load `defined_lib_features` query less

Hopefully addresses the perf regressions from #99212 (see #99905).

Re-structure the stability checks for library features to avoid calling `defined_lib_features` for any more crates than necessary for each of the implications or local feature attributes that need validation.

r? `@ghost` (just checking perf at first)

Author: bors

compiler/rustc_passes/src/stability.rs

@@ -962,58 +962,113 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
     remaining_lib_features.remove(&sym::libc);
     remaining_lib_features.remove(&sym::test);
 
-    // 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(());
-    let mut all_lib_features: FxHashMap<_, _> =
-        local_defined_features.to_vec().iter().map(|el| *el).collect();
-    let mut implications = tcx.stability_implications(rustc_hir::def_id::LOCAL_CRATE).clone();
-    for &cnum in tcx.crates(()) {
-        implications.extend(tcx.stability_implications(cnum));
-        all_lib_features.extend(tcx.defined_lib_features(cnum).iter().map(|el| *el));
-    }
-
-    // Check that every feature referenced by an `implied_by` exists (for features defined in the
-    // local crate).
-    for (implied_by, feature) in tcx.stability_implications(rustc_hir::def_id::LOCAL_CRATE) {
-        // Only `implied_by` needs to be checked, `feature` is guaranteed to exist.
-        if !all_lib_features.contains_key(implied_by) {
-            let span = local_defined_features
-                .stable
-                .get(feature)
-                .map(|(_, span)| span)
-                .or_else(|| local_defined_features.unstable.get(feature))
-                .expect("feature that implied another does not exist");
-            tcx.sess
-                .struct_span_err(
-                    *span,
-                    format!("feature `{implied_by}` implying `{feature}` does not exist"),
-                )
-                .emit();
-        }
-    }
-
-    if !remaining_lib_features.is_empty() {
-        for (feature, since) in all_lib_features.iter() {
+    /// For each feature in `defined_features`..
+    ///
+    /// - If it is in `remaining_lib_features` (those features with `#![feature(..)]` attributes in
+    ///   the current crate), check if it is stable (or partially stable) and thus an unnecessary
+    ///   attribute.
+    /// - If it is in `remaining_implications` (a feature that is referenced by an `implied_by`
+    ///   from the current crate), then remove it from the remaining implications.
+    ///
+    /// Once this function has been invoked for every feature (local crate and all extern crates),
+    /// then..
+    ///
+    /// - If features remain in `remaining_lib_features`, then the user has enabled a feature that
+    ///   does not exist.
+    /// - If features remain in `remaining_implications`, the `implied_by` refers to a feature that
+    ///   does not exist.
+    ///
+    /// By structuring the code in this way: checking the features defined from each crate one at a
+    /// time, less loading from metadata is performed and thus compiler performance is improved.
+    fn check_features<'tcx>(
+        tcx: TyCtxt<'tcx>,
+        remaining_lib_features: &mut FxIndexMap<&Symbol, Span>,
+        remaining_implications: &mut FxHashMap<Symbol, Symbol>,
+        defined_features: &[(Symbol, Option<Symbol>)],
+        all_implications: &FxHashMap<Symbol, Symbol>,
+    ) {
+        for (feature, since) in defined_features {
             if let Some(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) = implications.get(&feature) {
+                if let Some(implies) = all_implications.get(&feature) {
                     unnecessary_partially_stable_feature_lint(tcx, *span, *feature, *implies, *since);
                 } else {
                     unnecessary_stable_feature_lint(tcx, *span, *feature, *since);
                 }
+
             }
-            remaining_lib_features.remove(&feature);
-            if remaining_lib_features.is_empty() {
+            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);
+
+            if remaining_lib_features.is_empty() && remaining_implications.is_empty() {
                 break;
             }
         }
     }
 
+    // All local crate implications need to have the feature that implies it confirmed to exist.
+    let mut remaining_implications =
+        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();
+    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
+        // `remaining_lib_features`.
+        let mut all_implications = remaining_implications.clone();
+        for &cnum in tcx.crates(()) {
+            all_implications.extend(tcx.stability_implications(cnum));
+        }
+
+        check_features(
+            tcx,
+            &mut remaining_lib_features,
+            &mut remaining_implications,
+            local_defined_features.as_slice(),
+            &all_implications,
+        );
+
+        for &cnum in tcx.crates(()) {
+            if remaining_lib_features.is_empty() && remaining_implications.is_empty() {
+                break;
+            }
+            check_features(
+                tcx,
+                &mut remaining_lib_features,
+                &mut remaining_implications,
+                tcx.defined_lib_features(cnum).to_vec().as_slice(),
+                &all_implications,
+            );
+        }
+    }
+
     for (feature, span) in remaining_lib_features {
         struct_span_err!(tcx.sess, span, E0635, "unknown feature `{}`", feature).emit();
     }
 
+    for (implied_by, feature) in remaining_implications {
+        let local_defined_features = tcx.lib_features(());
+        let span = local_defined_features
+            .stable
+            .get(&feature)
+            .map(|(_, span)| span)
+            .or_else(|| local_defined_features.unstable.get(&feature))
+            .expect("feature that implied another does not exist");
+        tcx.sess
+            .struct_span_err(
+                *span,
+                format!("feature `{implied_by}` implying `{feature}` does not exist"),
+            )
+            .emit();
+    }
+
     // FIXME(#44232): the `used_features` table no longer exists, so we
     // don't lint about unused features. We should re-enable this one day!
 }