ensure that all publicly reachable const fn have const stability info

Author: RalfJung

compiler/rustc_attr/src/builtin.rs

@@ -16,9 +16,9 @@ use rustc_session::lint::BuiltinLintDiag;
 use rustc_session::lint::builtin::UNEXPECTED_CFGS;
 use rustc_session::parse::feature_err;
 use rustc_session::{RustcVersion, Session};
+use rustc_span::Span;
 use rustc_span::hygiene::Transparency;
 use rustc_span::symbol::{Symbol, kw, sym};
-use rustc_span::{DUMMY_SP, Span};
 
 use crate::fluent_generated;
 use crate::session_diagnostics::{self, IncorrectReprFormatGenericCause};
@@ -92,9 +92,7 @@ impl Stability {
 #[derive(HashStable_Generic)]
 pub struct ConstStability {
     pub level: StabilityLevel,
-    /// This can be `None` for functions that do not have an explicit const feature.
-    /// We still track them for recursive const stability checks.
-    pub feature: Option<Symbol>,
+    pub feature: Symbol,
     /// This is true iff the `const_stable_indirect` attribute is present.
     pub const_stable_indirect: bool,
     /// whether the function has a `#[rustc_promotable]` attribute
@@ -272,22 +270,19 @@ pub fn find_stability(
 
 /// Collects stability info from `rustc_const_stable`/`rustc_const_unstable`/`rustc_promotable`
 /// attributes in `attrs`. Returns `None` if no stability attributes are found.
-///
-/// `is_const_fn` indicates whether this is a function marked as `const`.
 pub fn find_const_stability(
     sess: &Session,
     attrs: &[Attribute],
     item_sp: Span,
-    is_const_fn: bool,
 ) -> Option<(ConstStability, Span)> {
     let mut const_stab: Option<(ConstStability, Span)> = None;
     let mut promotable = false;
-    let mut const_stable_indirect = None;
+    let mut const_stable_indirect = false;
 
     for attr in attrs {
         match attr.name_or_empty() {
             sym::rustc_promotable => promotable = true,
-            sym::rustc_const_stable_indirect => const_stable_indirect = Some(attr.span),
+            sym::rustc_const_stable_indirect => const_stable_indirect = true,
             sym::rustc_const_unstable => {
                 if const_stab.is_some() {
                     sess.dcx()
@@ -299,7 +294,7 @@ pub fn find_const_stability(
                     const_stab = Some((
                         ConstStability {
                             level,
-                            feature: Some(feature),
+                            feature,
                             const_stable_indirect: false,
                             promotable: false,
                         },
@@ -317,7 +312,7 @@ pub fn find_const_stability(
                     const_stab = Some((
                         ConstStability {
                             level,
-                            feature: Some(feature),
+                            feature,
                             const_stable_indirect: false,
                             promotable: false,
                         },
@@ -340,7 +335,7 @@ pub fn find_const_stability(
             }
         }
     }
-    if const_stable_indirect.is_some() {
+    if const_stable_indirect {
         match &mut const_stab {
             Some((stab, _)) => {
                 if stab.is_const_unstable() {
@@ -351,32 +346,13 @@ pub fn find_const_stability(
                     })
                 }
             }
-            _ => {}
+            _ => {
+                // This function has no const stability attribute, but has `const_stable_indirect`.
+                // We ignore that; unmarked functions are subject to recursive const stability
+                // checks by default so we do carry out the user's intent.
+            }
         }
     }
-    // Make sure if `const_stable_indirect` is present, that is recorded. Also make sure all `const
-    // fn` get *some* marker, since we are a staged_api crate and therefore will do recursive const
-    // stability checks for them. We need to do this because the default for whether an unmarked
-    // function enforces recursive stability differs between staged-api crates and force-unmarked
-    // crates: in force-unmarked crates, only functions *explicitly* marked `const_stable_indirect`
-    // enforce recursive stability. Therefore when `lookup_const_stability` is `None`, we have to
-    // assume the function does not have recursive stability. All functions that *do* have recursive
-    // stability must explicitly 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()) && const_stab.is_none() {
-        let c = ConstStability {
-            feature: None,
-            const_stable_indirect: const_stable_indirect.is_some(),
-            promotable: false,
-            level: StabilityLevel::Unstable {
-                reason: UnstableReason::Default,
-                issue: None,
-                is_soft: false,
-                implied_by: None,
-            },
-        };
-        const_stab = Some((c, const_stable_indirect.unwrap_or(DUMMY_SP)));
-    }
 
     const_stab
 }
@@ -394,7 +370,7 @@ pub fn unmarked_crate_const_stab(
     let const_stable_indirect =
         attrs.iter().any(|a| a.name_or_empty() == sym::rustc_const_stable_indirect);
     ConstStability {
-        feature: Some(regular_stab.feature),
+        feature: regular_stab.feature,
         const_stable_indirect,
         promotable: false,
         level: regular_stab.level,

compiler/rustc_const_eval/src/check_consts/check.rs

@@ -736,24 +736,26 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
 
                 // Intrinsics are language primitives, not regular calls, so treat them separately.
                 if let Some(intrinsic) = tcx.intrinsic(callee) {
+                    if !tcx.is_const_fn(callee) {
+                        // Non-const intrinsic.
+                        self.check_op(ops::IntrinsicNonConst { name: intrinsic.name });
+                        // If we allowed this, we're in miri-unleashed mode, so we might
+                        // as well skip the remaining checks.
+                        return;
+                    }
                     // We use `intrinsic.const_stable` to determine if this can be safely exposed to
                     // stable code, rather than `const_stable_indirect`. This is to make
                     // `#[rustc_const_stable_indirect]` an attribute that is always safe to add.
                     // We also ask is_safe_to_expose_on_stable_const_fn; this determines whether the intrinsic
                     // fallback body is safe to expose on stable.
                     let is_const_stable = intrinsic.const_stable
                         || (!intrinsic.must_be_overridden
-                            && tcx.is_const_fn(callee)
                             && is_safe_to_expose_on_stable_const_fn(tcx, callee));
                     match tcx.lookup_const_stability(callee) {
                         None => {
-                            // Non-const intrinsic.
-                            self.check_op(ops::IntrinsicNonConst { name: intrinsic.name });
-                        }
-                        Some(ConstStability { feature: None, .. }) => {
-                            // Intrinsic does not need a separate feature gate (we rely on the
-                            // regular stability checker). However, we have to worry about recursive
-                            // const stability.
+                            // 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.
                             if !is_const_stable && self.enforce_recursive_const_stability() {
                                 self.dcx().emit_err(errors::UnmarkedIntrinsicExposed {
                                     span: self.span,
@@ -762,8 +764,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                             }
                         }
                         Some(ConstStability {
-                            feature: Some(feature),
                             level: StabilityLevel::Unstable { .. },
+                            feature,
                             ..
                         }) => {
                             self.check_op(ops::IntrinsicUnstable {
@@ -802,7 +804,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                     Some(ConstStability { level: StabilityLevel::Stable { .. }, .. }) => {
                         // All good.
                     }
-                    None | Some(ConstStability { feature: None, .. }) => {
+                    None => {
                         // 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.
@@ -816,8 +818,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                         }
                     }
                     Some(ConstStability {
-                        feature: Some(feature),
                         level: StabilityLevel::Unstable { implied_by: implied_feature, .. },
+                        feature,
                         ..
                     }) => {
                         // An unstable const fn with a feature gate.

compiler/rustc_const_eval/src/check_consts/mod.rs

@@ -110,14 +110,15 @@ pub fn is_safe_to_expose_on_stable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> b
 
     match tcx.lookup_const_stability(def_id) {
         None => {
-            // Only marked functions can be trusted. Note that this may be a function in a
-            // non-staged-API crate where no recursive checks were done!
-            false
+            // In a `staged_api` crate, we do enforce recursive const stability for all unmarked
+            // functions, so we can trust local functions. But in another crate we don't know which
+            // rules were applied, so we can't trust that.
+            def_id.is_local() && tcx.features().staged_api()
         }
         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.feature.is_none() || stab.const_stable_indirect
+            // We consider things safe-to-expose if they are stable or if they are marked as
+            // `const_stable_indirect`.
+            stab.is_const_stable() || stab.const_stable_indirect
         }
     }
 }

compiler/rustc_expand/src/base.rs

@@ -866,9 +866,7 @@ impl SyntaxExtension {
             })
             .unwrap_or_else(|| (None, helper_attrs));
         let stability = attr::find_stability(sess, attrs, span);
-        // We set `is_const_fn` false to avoid getting any implicit const stability.
-        let const_stability =
-            attr::find_const_stability(sess, attrs, span, /* is_const_fn */ false);
+        let const_stability = attr::find_const_stability(sess, attrs, span);
         let body_stability = attr::find_body_stability(sess, attrs);
         if let Some((_, sp)) = const_stability {
             sess.dcx().emit_err(errors::MacroConstStability {