Auto merge of #131321 - RalfJung:feature-activation, r=nnethercote

terminology: #[feature] *enables* a feature (instead of "declaring" or "activating" it)

Mostly, we currently call a feature that has a corresponding `#[feature(name)]` attribute in the current crate a "declared" feature. I think that is confusing as it does not align with what "declaring" usually means. Furthermore, we *also* refer to `#[stable]`/`#[unstable]` as *declaring* a feature (e.g. in [these diagnostics](https://github.com/rust-lang/rust/blob/f25e5abea229a6b6aa77b45e21cb784e785c6040/compiler/rustc_passes/messages.ftl#L297-L301)), which aligns better with what "declaring" usually means. To make things worse, the functions  `tcx.features().active(...)` and  `tcx.features().declared(...)` both exist and they are doing almost the same thing (testing whether a corresponding `#[feature(name)]`  exists) except that `active` would ICE if the feature is not an unstable lang feature. On top of this, the callback when a feature is activated/declared is called `set_enabled`, and many comments also talk about "enabling" a feature.

So really, our terminology is just a mess.

I would suggest we use "declaring a feature" for saying that something is/was guarded by a feature (e.g. `#[stable]`/`#[unstable]`), and "enabling a feature" for  `#[feature(name)]`. This PR implements that.

Author: bors

compiler/rustc_ast_passes/src/feature_gate.rs

@@ -600,59 +600,61 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session, features: &Features) {
 }
 
 fn maybe_stage_features(sess: &Session, features: &Features, krate: &ast::Crate) {
-    // checks if `#![feature]` has been used to enable any lang feature
-    // does not check the same for lib features unless there's at least one
-    // declared lang feature
-    if !sess.opts.unstable_features.is_nightly_build() {
-        if features.declared_features.is_empty() {
-            return;
-        }
-        for attr in krate.attrs.iter().filter(|attr| attr.has_name(sym::feature)) {
-            let mut err = errors::FeatureOnNonNightly {
-                span: attr.span,
-                channel: option_env!("CFG_RELEASE_CHANNEL").unwrap_or("(unknown)"),
-                stable_features: vec![],
-                sugg: None,
-            };
-
-            let mut all_stable = true;
-            for ident in
-                attr.meta_item_list().into_iter().flatten().flat_map(|nested| nested.ident())
-            {
-                let name = ident.name;
-                let stable_since = features
-                    .declared_lang_features
-                    .iter()
-                    .flat_map(|&(feature, _, since)| if feature == name { since } else { None })
-                    .next();
-                if let Some(since) = stable_since {
-                    err.stable_features.push(errors::StableFeature { name, since });
-                } else {
-                    all_stable = false;
-                }
-            }
-            if all_stable {
-                err.sugg = Some(attr.span);
+    // checks if `#![feature]` has been used to enable any feature.
+    if sess.opts.unstable_features.is_nightly_build() {
+        return;
+    }
+    if features.enabled_features().is_empty() {
+        return;
+    }
+    let mut errored = false;
+    for attr in krate.attrs.iter().filter(|attr| attr.has_name(sym::feature)) {
+        // `feature(...)` used on non-nightly. This is definitely an error.
+        let mut err = errors::FeatureOnNonNightly {
+            span: attr.span,
+            channel: option_env!("CFG_RELEASE_CHANNEL").unwrap_or("(unknown)"),
+            stable_features: vec![],
+            sugg: None,
+        };
+
+        let mut all_stable = true;
+        for ident in attr.meta_item_list().into_iter().flatten().flat_map(|nested| nested.ident()) {
+            let name = ident.name;
+            let stable_since = features
+                .enabled_lang_features()
+                .iter()
+                .flat_map(|&(feature, _, since)| if feature == name { since } else { None })
+                .next();
+            if let Some(since) = stable_since {
+                err.stable_features.push(errors::StableFeature { name, since });
+            } else {
+                all_stable = false;
             }
-            sess.dcx().emit_err(err);
         }
+        if all_stable {
+            err.sugg = Some(attr.span);
+        }
+        sess.dcx().emit_err(err);
+        errored = true;
     }
+    // Just make sure we actually error if anything is listed in `enabled_features`.
+    assert!(errored);
 }
 
 fn check_incompatible_features(sess: &Session, features: &Features) {
-    let declared_features = features
-        .declared_lang_features
+    let enabled_features = features
+        .enabled_lang_features()
         .iter()
         .copied()
         .map(|(name, span, _)| (name, span))
-        .chain(features.declared_lib_features.iter().copied());
+        .chain(features.enabled_lib_features().iter().copied());
 
     for (f1, f2) in rustc_feature::INCOMPATIBLE_FEATURES
         .iter()
-        .filter(|&&(f1, f2)| features.active(f1) && features.active(f2))
+        .filter(|&&(f1, f2)| features.enabled(f1) && features.enabled(f2))
     {
-        if let Some((f1_name, f1_span)) = declared_features.clone().find(|(name, _)| name == f1) {
-            if let Some((f2_name, f2_span)) = declared_features.clone().find(|(name, _)| name == f2)
+        if let Some((f1_name, f1_span)) = enabled_features.clone().find(|(name, _)| name == f1) {
+            if let Some((f2_name, f2_span)) = enabled_features.clone().find(|(name, _)| name == f2)
             {
                 let spans = vec![f1_span, f2_span];
                 sess.dcx().emit_err(errors::IncompatibleFeatures {
@@ -672,7 +674,7 @@ fn check_new_solver_banned_features(sess: &Session, features: &Features) {
 
     // Ban GCE with the new solver, because it does not implement GCE correctly.
     if let Some(&(_, gce_span, _)) = features
-        .declared_lang_features
+        .enabled_lang_features()
         .iter()
         .find(|&&(feat, _, _)| feat == sym::generic_const_exprs)
     {

compiler/rustc_const_eval/src/check_consts/check.rs

@@ -276,7 +276,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
         let gate = match op.status_in_item(self.ccx) {
             Status::Allowed => return,
 
-            Status::Unstable(gate) if self.tcx.features().active(gate) => {
+            Status::Unstable(gate) if self.tcx.features().enabled(gate) => {
                 let unstable_in_stable = self.ccx.is_const_stable_const_fn()
                     && !super::rustc_allow_const_fn_unstable(self.tcx, self.def_id(), gate);
                 if unstable_in_stable {
@@ -700,10 +700,10 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                     // Calling an unstable function *always* requires that the corresponding gate
                     // (or implied gate) be enabled, even if the function has
                     // `#[rustc_allow_const_fn_unstable(the_gate)]`.
-                    let gate_declared = |gate| tcx.features().declared(gate);
-                    let feature_gate_declared = gate_declared(gate);
-                    let implied_gate_declared = implied_by.is_some_and(gate_declared);
-                    if !feature_gate_declared && !implied_gate_declared {
+                    let gate_enabled = |gate| tcx.features().enabled(gate);
+                    let feature_gate_enabled = gate_enabled(gate);
+                    let implied_gate_enabled = implied_by.is_some_and(gate_enabled);
+                    if !feature_gate_enabled && !implied_gate_enabled {
                         self.check_op(ops::FnCallUnstable(callee, Some(gate)));
                         return;
                     }
@@ -717,7 +717,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
 
                     // Otherwise, we are something const-stable calling a const-unstable fn.
                     if super::rustc_allow_const_fn_unstable(tcx, caller, gate) {
-                        trace!("rustc_allow_const_fn_unstable gate active");
+                        trace!("rustc_allow_const_fn_unstable gate enabled");
                         return;
                     }
 

compiler/rustc_error_codes/src/error_codes/E0636.md

@@ -1,9 +1,9 @@
-A `#![feature]` attribute was declared multiple times.
+The same feature is enabled multiple times with `#![feature]` attributes
 
 Erroneous code example:
 
 ```compile_fail,E0636
 #![allow(stable_features)]
 #![feature(rust1)]
-#![feature(rust1)] // error: the feature `rust1` has already been declared
+#![feature(rust1)] // error: the feature `rust1` has already been enabled
 ```

compiler/rustc_error_codes/src/error_codes/E0705.md

@@ -1,6 +1,6 @@
 #### Note: this error code is no longer emitted by the compiler.
 
-A `#![feature]` attribute was declared for a feature that is stable in the
+A `#![feature]` attribute was used for a feature that is stable in the
 current edition, but not in all editions.
 
 Erroneous code example:

compiler/rustc_expand/src/config.rs

@@ -52,7 +52,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute], crate_name: Symbol) -
 
     let mut features = Features::default();
 
-    // Process all features declared in the code.
+    // Process all features enabled in the code.
     for attr in krate_attrs {
         for mi in feature_list(attr) {
             let name = match mi.ident() {
@@ -76,7 +76,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute], crate_name: Symbol) -
                 }
             };
 
-            // If the declared feature has been removed, issue an error.
+            // If the enabled feature has been removed, issue an error.
             if let Some(f) = REMOVED_FEATURES.iter().find(|f| name == f.feature.name) {
                 sess.dcx().emit_err(FeatureRemoved {
                     span: mi.span(),
@@ -85,14 +85,14 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute], crate_name: Symbol) -
                 continue;
             }
 
-            // If the declared feature is stable, record it.
+            // If the enabled feature is stable, record it.
             if let Some(f) = ACCEPTED_FEATURES.iter().find(|f| name == f.name) {
                 let since = Some(Symbol::intern(f.since));
-                features.set_declared_lang_feature(name, mi.span(), since);
+                features.set_enabled_lang_feature(name, mi.span(), since, None);
                 continue;
             }
 
-            // If `-Z allow-features` is used and the declared feature is
+            // If `-Z allow-features` is used and the enabled feature is
             // unstable and not also listed as one of the allowed features,
             // issue an error.
             if let Some(allowed) = sess.opts.unstable_opts.allow_features.as_ref() {
@@ -102,9 +102,8 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute], crate_name: Symbol) -
                 }
             }
 
-            // If the declared feature is unstable, record it.
+            // If the enabled feature is unstable, record it.
             if let Some(f) = UNSTABLE_FEATURES.iter().find(|f| name == f.feature.name) {
-                (f.set_enabled)(&mut features);
                 // When the ICE comes from core, alloc or std (approximation of the standard
                 // library), there's a chance that the person hitting the ICE may be using
                 // -Zbuild-std or similar with an untested target. The bug is probably in the
@@ -115,13 +114,13 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute], crate_name: Symbol) -
                 {
                     sess.using_internal_features.store(true, std::sync::atomic::Ordering::Relaxed);
                 }
-                features.set_declared_lang_feature(name, mi.span(), None);
+                features.set_enabled_lang_feature(name, mi.span(), None, Some(f));
                 continue;
             }
 
-            // Otherwise, the feature is unknown. Record it as a lib feature.
-            // It will be checked later.
-            features.set_declared_lib_feature(name, mi.span());
+            // Otherwise, the feature is unknown. Enable it as a lib feature.
+            // It will be checked later whether the feature really exists.
+            features.set_enabled_lib_feature(name, mi.span());
 
             // Similar to above, detect internal lib features to suppress
             // the ICE message that asks for a report.

compiler/rustc_expand/src/mbe/quoted.rs

@@ -119,7 +119,7 @@ pub(super) fn parse(
     result
 }
 
-/// Asks for the `macro_metavar_expr` feature if it is not already declared
+/// Asks for the `macro_metavar_expr` feature if it is not enabled
 fn maybe_emit_macro_metavar_expr_feature(features: &Features, sess: &Session, span: Span) {
     if !features.macro_metavar_expr {
         let msg = "meta-variable expressions are unstable";

compiler/rustc_feature/src/unstable.rs

@@ -8,7 +8,7 @@ use super::{Feature, to_nonzero};
 
 pub struct UnstableFeature {
     pub feature: Feature,
-    pub set_enabled: fn(&mut Features),
+    set_enabled: fn(&mut Features),
 }
 
 #[derive(PartialEq)]
@@ -54,61 +54,85 @@ macro_rules! declare_features {
         #[derive(Clone, Default, Debug)]
         pub struct Features {
             /// `#![feature]` attrs for language features, for error reporting.
-            /// "declared" here means that the feature is actually enabled in the current crate.
-            pub declared_lang_features: Vec<(Symbol, Span, Option<Symbol>)>,
+            enabled_lang_features: Vec<(Symbol, Span, Option<Symbol>)>,
             /// `#![feature]` attrs for non-language (library) features.
-            /// "declared" here means that the feature is actually enabled in the current crate.
-            pub declared_lib_features: Vec<(Symbol, Span)>,
-            /// `declared_lang_features` + `declared_lib_features`.
-            pub declared_features: FxHashSet<Symbol>,
-            /// Active state of individual features (unstable only).
+            enabled_lib_features: Vec<(Symbol, Span)>,
+            /// `enabled_lang_features` + `enabled_lib_features`.
+            enabled_features: FxHashSet<Symbol>,
+            /// State of individual features (unstable lang features only).
+            /// This is `true` if and only if the corresponding feature is listed in `enabled_lang_features`.
             $(
                 $(#[doc = $doc])*
                 pub $feature: bool
             ),+
         }
 
         impl Features {
-            pub fn set_declared_lang_feature(
+            pub fn set_enabled_lang_feature(
                 &mut self,
-                symbol: Symbol,
+                name: Symbol,
                 span: Span,
-                since: Option<Symbol>
+                since: Option<Symbol>,
+                feature: Option<&UnstableFeature>,
             ) {
-                self.declared_lang_features.push((symbol, span, since));
-                self.declared_features.insert(symbol);
+                self.enabled_lang_features.push((name, span, since));
+                self.enabled_features.insert(name);
+                if let Some(feature) = feature {
+                    assert_eq!(feature.feature.name, name);
+                    (feature.set_enabled)(self);
+                } else {
+                    // Ensure we don't skip a `set_enabled` call.
+                    debug_assert!(UNSTABLE_FEATURES.iter().find(|f| name == f.feature.name).is_none());
+                }
             }
 
-            pub fn set_declared_lib_feature(&mut self, symbol: Symbol, span: Span) {
-                self.declared_lib_features.push((symbol, span));
-                self.declared_features.insert(symbol);
+            pub fn set_enabled_lib_feature(&mut self, name: Symbol, span: Span) {
+                self.enabled_lib_features.push((name, span));
+                self.enabled_features.insert(name);
+                // Ensure we don't skip a `set_enabled` call.
+                debug_assert!(UNSTABLE_FEATURES.iter().find(|f| name == f.feature.name).is_none());
             }
 
-            /// This is intended for hashing the set of active features.
+            /// This is intended for hashing the set of enabled language features.
             ///
             /// The expectation is that this produces much smaller code than other alternatives.
             ///
             /// Note that the total feature count is pretty small, so this is not a huge array.
             #[inline]
-            pub fn all_features(&self) -> [u8; NUM_FEATURES] {
+            pub fn all_lang_features(&self) -> [u8; NUM_FEATURES] {
                 [$(self.$feature as u8),+]
             }
 
-            /// Is the given feature explicitly declared, i.e. named in a
-            /// `#![feature(...)]` within the code?
-            pub fn declared(&self, feature: Symbol) -> bool {
-                self.declared_features.contains(&feature)
+            pub fn enabled_lang_features(&self) -> &Vec<(Symbol, Span, Option<Symbol>)> {
+                &self.enabled_lang_features
             }
 
-            /// Is the given feature active (enabled by the user)?
-            ///
-            /// Panics if the symbol doesn't correspond to a declared feature.
-            pub fn active(&self, feature: Symbol) -> bool {
-                match feature {
-                    $( sym::$feature => self.$feature, )*
+            pub fn enabled_lib_features(&self) -> &Vec<(Symbol, Span)> {
+                &self.enabled_lib_features
+            }
 
-                    _ => panic!("`{}` was not listed in `declare_features`", feature),
+            pub fn enabled_features(&self) -> &FxHashSet<Symbol> {
+                &self.enabled_features
+            }
+
+            /// Is the given feature enabled (via `#[feature(...)]`)?
+            pub fn enabled(&self, feature: Symbol) -> bool {
+                let e = self.enabled_features.contains(&feature);
+                if cfg!(debug_assertions) {
+                    // Ensure this matches `self.$feature`, if that exists.
+                    let e2 = match feature {
+                        $( sym::$feature => Some(self.$feature), )*
+                        _ => None,
+                    };
+                    if let Some(e2) = e2 {
+                        assert_eq!(
+                            e, e2,
+                            "mismatch in feature state for `{feature}`: \
+                            `enabled_features` says {e} but `self.{feature}` says {e2}"
+                        );
+                    }
                 }
+                e
             }
 
             /// Some features are known to be incomplete and using them is likely to have
@@ -119,8 +143,11 @@ macro_rules! declare_features {
                     $(
                         sym::$feature => status_to_enum!($status) == FeatureStatus::Incomplete,
                     )*
-                    // Accepted/removed features aren't in this file but are never incomplete.
-                    _ if self.declared_features.contains(&feature) => false,
+                    _ if self.enabled_features.contains(&feature) => {
+                        // Accepted/removed features and library features aren't in this file but
+                        // are never incomplete.
+                        false
+                    }
                     _ => panic!("`{}` was not listed in `declare_features`", feature),
                 }
             }
@@ -132,7 +159,7 @@ macro_rules! declare_features {
                     $(
                         sym::$feature => status_to_enum!($status) == FeatureStatus::Internal,
                     )*
-                    _ if self.declared_features.contains(&feature) => {
+                    _ if self.enabled_features.contains(&feature) => {
                         // This could be accepted/removed, or a libs feature.
                         // Accepted/removed features aren't in this file but are never internal
                         // (a removed feature might have been internal, but that's now irrelevant).

compiler/rustc_lint/src/builtin.rs

@@ -2288,10 +2288,10 @@ impl EarlyLintPass for IncompleteInternalFeatures {
     fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &ast::Crate) {
         let features = cx.builder.features();
         features
-            .declared_lang_features
+            .enabled_lang_features()
             .iter()
             .map(|(name, span, _)| (name, span))
-            .chain(features.declared_lib_features.iter().map(|(name, span)| (name, span)))
+            .chain(features.enabled_lib_features().iter().map(|(name, span)| (name, span)))
             .filter(|(&name, _)| features.incomplete(name) || features.internal(name))
             .for_each(|(&name, &span)| {
                 if features.incomplete(name) {