Clarify/fix const-unstable semantics in the presence of multiple unstable features

dianne · dianne · commit 5a1fd556b93e · 2024-10-28T16:25:36.000-07:00
diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs
@@ -96,6 +96,7 @@ impl Stability {
 }
 
 /// Represents the `#[rustc_const_unstable]` and `#[rustc_const_stable]` attributes.
+/// For details see [the dev guide](https://rustc-dev-guide.rust-lang.org/stability.html#rustc_const_unstable).
 #[derive(Encodable, Decodable, Clone, Debug, PartialEq, Eq, Hash)]
 #[derive(HashStable_Generic)]
 pub struct ConstStability {
@@ -115,8 +116,8 @@ impl ConstStability {
         self.level.is_const_stable()
     }
 
-    pub fn is_implicit(&self) -> bool {
-        self.level.is_implicit()
+    pub fn is_implicit_unstable(&self) -> bool {
+        self.level.is_implicit_unstable()
     }
 
     pub fn unstable_features(&self) -> impl Iterator<Item = Symbol> + use<'_> {
@@ -154,7 +155,6 @@ pub enum StabilityLevel {
 }
 
 /// The available const-stability levels for const functions.
-/// For details see [#131349](https://github.com/rust-lang/rust/pull/131349).
 #[derive(Encodable, Decodable, PartialEq, Clone, Debug, Eq, Hash)]
 #[derive(HashStable_Generic)]
 pub enum ConstStabilityLevel {
@@ -169,7 +169,7 @@ pub enum ConstStabilityLevel {
     },
     /// For functions with no explicit const-stability attribute that require checking recursive
     /// const stability. This is either an unmarked const fn or a `const_stable_indirect` intrinsic.
-    Implicit,
+    ImplicitUnstable,
 }
 
 /// Rust release in which a feature is stabilized.
@@ -224,15 +224,15 @@ impl StabilityLevel {
 
 impl ConstStabilityLevel {
     pub fn is_const_unstable(&self) -> bool {
-        matches!(self, ConstStabilityLevel::Unstable { .. })
+        matches!(self, ConstStabilityLevel::Unstable { .. } | ConstStabilityLevel::ImplicitUnstable)
     }
 
     pub fn is_const_stable(&self) -> bool {
         matches!(self, ConstStabilityLevel::Stable { .. })
     }
 
-    pub fn is_implicit(&self) -> bool {
-        matches!(self, ConstStabilityLevel::Implicit)
+    pub fn is_implicit_unstable(&self) -> bool {
+        matches!(self, ConstStabilityLevel::ImplicitUnstable)
     }
 
     pub fn unstable_features(&self) -> impl Iterator<Item = Symbol> + use<'_> {
@@ -429,10 +429,10 @@ pub fn find_const_stability(
         // record this, and so that's what we do for all `const fn` in a staged_api crate.
         if is_const_fn || const_stable_indirect.is_some() {
             stab_spans = ConstStabilitySpans(smallvec![(
-                ConstStabilityLevel::Implicit,
+                ConstStabilityLevel::ImplicitUnstable,
                 const_stable_indirect.unwrap_or(DUMMY_SP)
             )]);
-            ConstStabilityLevel::Implicit
+            ConstStabilityLevel::ImplicitUnstable
         } else {
             return None;
         }
diff --git a/compiler/rustc_const_eval/src/check_consts/check.rs b/compiler/rustc_const_eval/src/check_consts/check.rs
@@ -23,7 +23,6 @@ use rustc_mir_dataflow::storage::always_storage_live_locals;
 use rustc_span::{DUMMY_SP, Span, Symbol, sym};
 use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
 use rustc_trait_selection::traits::{self, ObligationCauseCode, ObligationCtxt};
-use smallvec::SmallVec;
 use tracing::{debug, instrument, trace};
 
 use super::ops::{self, NonConstOp, Status};
@@ -720,7 +719,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                             self.check_op(ops::IntrinsicNonConst { name: intrinsic.name });
                         }
                         Some(ConstStability {
-                            level: ConstStabilityLevel::Implicit,
+                            level: ConstStabilityLevel::ImplicitUnstable,
                             const_stable_indirect,
                             ..
                         }) => {
@@ -776,7 +775,10 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                     Some(ConstStability { level: ConstStabilityLevel::Stable { .. }, .. }) => {
                         // All good.
                     }
-                    None | Some(ConstStability { level: ConstStabilityLevel::Implicit, .. }) => {
+                    None
+                    | Some(ConstStability {
+                        level: ConstStabilityLevel::ImplicitUnstable, ..
+                    }) => {
                         // This doesn't need a separate const-stability check -- const-stability equals
                         // regular stability, and regular stability is checked separately.
                         // However, we *do* have to worry about *recursive* const stability.
@@ -799,43 +801,33 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
 
                         // We only honor `span.allows_unstable` aka `#[allow_internal_unstable]` if
                         // the callee is safe to expose, to avoid bypassing recursive stability.
-                        let is_allowed_unstable = |u: &rustc_attr::Unstability| {
-                            callee_safe_to_expose_on_stable
-                                && (self.span.allows_unstable(u.feature)
-                                    || u.implied_by.is_some_and(|f| self.span.allows_unstable(f)))
-                        };
-                        let mut needs_check =
-                            unstables.iter().filter(|u| !is_allowed_unstable(u)).peekable();
-                        if needs_check.peek().is_none() {
+                        if callee_safe_to_expose_on_stable
+                            && unstables.iter().all(|u| {
+                                self.span.allows_unstable(u.feature)
+                                    || u.implied_by.is_some_and(|f| self.span.allows_unstable(f))
+                            })
+                        {
                             return;
                         }
 
                         // We can't use `check_op` to check whether the feature is enabled because
                         // the logic is a bit different than elsewhere: local functions don't need
                         // the feature gate, and there might be an "implied" gate that also suffices
                         // to allow this.
-                        let is_feature_enabled = |u: &rustc_attr::Unstability| {
-                            callee.is_local()
-                                || tcx.features().enabled(u.feature)
-                                || u.implied_by.is_some_and(|f| tcx.features().enabled(f))
-                        };
+                        let features_enabled = callee.is_local()
+                            || unstables.iter().all(|u| {
+                                tcx.features().enabled(u.feature)
+                                    || u.implied_by.is_some_and(|f| tcx.features().enabled(f))
+                            });
                         // We do *not* honor this if we are in the "danger zone": we have to enforce
                         // recursive const-stability and the callee is not safe-to-expose. In that
                         // case we need `check_op` to do the check.
                         let danger_zone = !callee_safe_to_expose_on_stable
                             && self.enforce_recursive_const_stability();
-                        let missing_features: SmallVec<[_; 1]> = if danger_zone {
-                            needs_check.map(|u| u.into()).collect()
-                        } else {
-                            needs_check
-                                .filter(|u| !is_feature_enabled(u))
-                                .map(|u| u.into())
-                                .collect()
-                        };
-                        if !missing_features.is_empty() {
+                        if danger_zone || !features_enabled {
                             self.check_op(ops::FnCallUnstable {
                                 def_id: callee,
-                                features: missing_features,
+                                features: unstables.iter().map(|u| u.into()).collect(),
                                 reason: *reason,
                                 safe_to_expose_on_stable: callee_safe_to_expose_on_stable,
                             });
diff --git a/compiler/rustc_const_eval/src/check_consts/mod.rs b/compiler/rustc_const_eval/src/check_consts/mod.rs
@@ -123,7 +123,7 @@ pub fn is_safe_to_expose_on_stable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> b
         Some(stab) => {
             // We consider things safe-to-expose if they are stable, if they don't have any explicit
             // const stability attribute, or if they are marked as `const_stable_indirect`.
-            stab.is_const_stable() || stab.is_implicit() || stab.const_stable_indirect
+            stab.is_const_stable() || stab.is_implicit_unstable() || stab.const_stable_indirect
         }
     }
 }
diff --git a/compiler/rustc_const_eval/src/check_consts/ops.rs b/compiler/rustc_const_eval/src/check_consts/ops.rs
@@ -326,15 +326,19 @@ impl<'tcx> NonConstOp<'tcx> for FnCallUnstable {
     }
 
     fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> Diag<'tcx> {
+        // Only report the features that aren't already enabled.
+        let missing_features = Vec::from_iter(
+            self.features.iter().filter(|d| !ccx.tcx.features().enabled(d.feature)).copied(),
+        );
         ccx.dcx().create_err(errors::UnstableConstFn {
             span,
             def_path: ccx.tcx.def_path_str(self.def_id),
-            features: stability::unstable_message(&self.features, self.reason.to_opt_reason())
+            features: stability::unstable_message(&missing_features, self.reason.to_opt_reason())
                 .hide_features_on_nightly(&ccx.tcx.sess),
-            issues: stability::unstable_issues(&self.features),
+            issues: stability::unstable_issues(&missing_features),
             nightly_subdiags: stability::unstable_nightly_subdiags(
                 &ccx.tcx.sess,
-                &self.features,
+                &missing_features,
                 None,
             ),
         })
@@ -378,15 +382,19 @@ impl<'tcx> NonConstOp<'tcx> for IntrinsicUnstable {
     }
 
     fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> Diag<'tcx> {
+        // Only report the features that aren't already enabled.
+        let missing_features = Vec::from_iter(
+            self.features.iter().filter(|d| !ccx.tcx.features().enabled(d.feature)).copied(),
+        );
         ccx.dcx().create_err(errors::UnstableIntrinsic {
             span,
             name: self.name,
-            features: stability::unstable_message(&self.features, self.reason.to_opt_reason())
+            features: stability::unstable_message(&missing_features, self.reason.to_opt_reason())
                 .hide_features_on_nightly(&ccx.tcx.sess),
-            issues: stability::unstable_issues(&self.features),
+            issues: stability::unstable_issues(&missing_features),
             nightly_subdiags: stability::unstable_nightly_subdiags(
                 &ccx.tcx.sess,
-                &self.features,
+                &missing_features,
                 None,
             ),
         })
diff --git a/compiler/rustc_middle/src/middle/stability.rs b/compiler/rustc_middle/src/middle/stability.rs
@@ -362,7 +362,7 @@ pub enum EvalResult {
 }
 
 /// An instance of a disabled feature required for an unstable item
-#[derive(Debug)]
+#[derive(Clone, Copy, Debug)]
 pub struct EvalDenial {
     pub feature: Symbol,
     pub issue: Option<NonZero<u32>>,
diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs
@@ -614,7 +614,7 @@ impl<'tcx> MissingStabilityAnnotations<'tcx> {
         let is_stable =
             self.tcx.lookup_stability(def_id).is_some_and(|stability| stability.level.is_stable());
         let missing_const_stability_attribute =
-            self.tcx.lookup_const_stability(def_id).is_none_or(|s| s.is_implicit());
+            self.tcx.lookup_const_stability(def_id).is_none_or(|s| s.is_implicit_unstable());
 
         if is_const && is_stable && missing_const_stability_attribute {
             let descr = self.tcx.def_descr(def_id.to_def_id());
diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
@@ -1701,14 +1701,12 @@ pub(crate) fn print_constness_with_space(
             | (_, None)
             // ...or when const unstable, but overall unstable too...
             | (None, Some(ConstStability { level: ConstStabilityLevel::Unstable { .. }, .. }))
-            // ...or when a const stability level was not explicitly provided, and overall unstable
-            | (None, Some(ConstStability { level: ConstStabilityLevel::Implicit, .. })) => {
+            | (None, Some(ConstStability { level: ConstStabilityLevel::ImplicitUnstable, .. })) => {
                 "const "
             }
             // const unstable (and overall stable)...
             (Some(_), Some(ConstStability { level: ConstStabilityLevel::Unstable { .. }, .. }))
-            // ...or without an explicit const stability level (and overall stable)
-            | (Some(_), Some(ConstStability { level: ConstStabilityLevel::Implicit, .. })) => "",
+            | (Some(_), Some(ConstStability { level: ConstStabilityLevel::ImplicitUnstable, .. })) => "",
         },
         // not const
         hir::Constness::NotConst => "",
diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
@@ -1041,7 +1041,7 @@ fn render_stability_since_raw_with_extra(
                 Some((String::from("const unstable"), format!("const: {unstable}")))
             }
         }
-        Some(ConstStability { level: ConstStabilityLevel::Implicit, .. }) => {
+        Some(ConstStability { level: ConstStabilityLevel::ImplicitUnstable, .. }) => {
             // No explicit const-stability annotation was provided. Treat it as const-unstable.
             if stable_version.is_none() {
                 None

Original file line number	Diff line number	Diff line change
`@@ -123,7 +123,7 @@ pub fn is_safe_to_expose_on_stable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> b`
`123`	`123`	`Some(stab) => {`
`124`	`124`	`// We consider things safe-to-expose if they are stable, if they don't have any explicit`
`125`	`125`	// const stability attribute, or if they are marked as `const_stable_indirect`.
`126`		`- stab.is_const_stable() \|\| stab.is_implicit() \|\| stab.const_stable_indirect`
	`126`	`+ stab.is_const_stable() \|\| stab.is_implicit_unstable() \|\| stab.const_stable_indirect`
`127`	`127`	`}`
`128`	`128`	`}`
`129`	`129`	`}`
Original file line number	Diff line number	Diff line change
`@@ -362,7 +362,7 @@ pub enum EvalResult {`
`362`	`362`	`}`
`363`	`363`
`364`	`364`	`/// An instance of a disabled feature required for an unstable item`
`365`		`-#[derive(Debug)]`
	`365`	`+#[derive(Clone, Copy, Debug)]`
`366`	`366`	`pub struct EvalDenial {`
`367`	`367`	`pub feature: Symbol,`
`368`	`368`	`pub issue: Option<NonZero<u32>>,`
Original file line number	Diff line number	Diff line change
`@@ -1041,7 +1041,7 @@ fn render_stability_since_raw_with_extra(`
`1041`	`1041`	`Some((String::from("const unstable"), format!("const: {unstable}")))`
`1042`	`1042`	`}`
`1043`	`1043`	`}`
`1044`		`- Some(ConstStability { level: ConstStabilityLevel::Implicit, .. }) => {`
	`1044`	`+ Some(ConstStability { level: ConstStabilityLevel::ImplicitUnstable, .. }) => {`
`1045`	`1045`	`// No explicit const-stability annotation was provided. Treat it as const-unstable.`
`1046`	`1046`	`if stable_version.is_none() {`
`1047`	`1047`	`None`