Auto merge of rust-lang#132992 - RalfJung:check-consts-feature-gate, r=compiler-errors

check_consts: fix error requesting feature gate when that gate is not actually needed

When working on rust-lang/hashbrown#586 I noticed that the compiler asks for the `rustc_private` feature to be enabled if one forgets to set `rustc_const_stable_indirect` on a function -- but enabling `rustc_private` would not actually help. This fixes the diagnostics.

r? `@compiler-errors`

Author: bors

compiler/rustc_const_eval/src/check_consts/check.rs

@@ -272,9 +272,18 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
     /// context.
     pub fn check_op_spanned<O: NonConstOp<'tcx>>(&mut self, op: O, span: Span) {
         let gate = match op.status_in_item(self.ccx) {
-            Status::Unstable { gate, safe_to_expose_on_stable, is_function_call }
-                if self.tcx.features().enabled(gate) =>
-            {
+            Status::Unstable {
+                gate,
+                safe_to_expose_on_stable,
+                is_function_call,
+                gate_already_checked,
+            } if gate_already_checked || self.tcx.features().enabled(gate) => {
+                if gate_already_checked {
+                    assert!(
+                        !safe_to_expose_on_stable,
+                        "setting `gate_already_checked` without `safe_to_expose_on_stable` makes no sense"
+                    );
+                }
                 // Generally this is allowed since the feature gate is enabled -- except
                 // if this function wants to be safe-to-expose-on-stable.
                 if !safe_to_expose_on_stable
@@ -745,7 +754,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                             self.check_op(ops::IntrinsicUnstable {
                                 name: intrinsic.name,
                                 feature,
-                                const_stable: is_const_stable,
+                                const_stable_indirect: is_const_stable,
                             });
                         }
                         Some(ConstStability { level: StabilityLevel::Stable { .. }, .. }) => {
@@ -800,6 +809,12 @@ 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.
+                        // This is not ideal since it means the user sees an error, not the macro
+                        // author, but that's also the case if one forgets to set
+                        // `#[allow_internal_unstable]` in the first place. Note that this cannot be
+                        // integrated in the check below since we want to enforce
+                        // `callee_safe_to_expose_on_stable` even if
+                        // `!self.enforce_recursive_const_stability()`.
                         if (self.span.allows_unstable(feature)
                             || implied_feature.is_some_and(|f| self.span.allows_unstable(f)))
                             && callee_safe_to_expose_on_stable
@@ -830,15 +845,13 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                                     && issue == NonZero::new(27812)
                                     && self.tcx.sess.opts.unstable_opts.force_unstable_if_unmarked
                             };
-                        // 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();
-                        if danger_zone || !feature_enabled {
+                        // Even if the feature is enabled, we still need check_op to double-check
+                        // this if the callee is not safe to expose on stable.
+                        if !feature_enabled || !callee_safe_to_expose_on_stable {
                             self.check_op(ops::FnCallUnstable {
                                 def_id: callee,
                                 feature,
+                                feature_enabled,
                                 safe_to_expose_on_stable: callee_safe_to_expose_on_stable,
                             });
                         }

compiler/rustc_const_eval/src/check_consts/ops.rs

@@ -28,6 +28,9 @@ pub enum Status {
     Unstable {
         /// The feature that must be enabled to use this operation.
         gate: Symbol,
+        /// Whether the feature gate was already checked (because the logic is a bit more
+        /// complicated than just checking a single gate).
+        gate_already_checked: bool,
         /// Whether it is allowed to use this operation from stable `const fn`.
         /// This will usually be `false`.
         safe_to_expose_on_stable: bool,
@@ -82,6 +85,7 @@ impl<'tcx> NonConstOp<'tcx> for ConditionallyConstCall<'tcx> {
         // We use the `const_trait_impl` gate for all conditionally-const calls.
         Status::Unstable {
             gate: sym::const_trait_impl,
+            gate_already_checked: false,
             safe_to_expose_on_stable: false,
             // We don't want the "mark the callee as `#[rustc_const_stable_indirect]`" hint
             is_function_call: false,
@@ -330,19 +334,24 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
 pub(crate) struct FnCallUnstable {
     pub def_id: DefId,
     pub feature: Symbol,
+    /// If this is true, then the feature is enabled, but we need to still check if it is safe to
+    /// expose on stable.
+    pub feature_enabled: bool,
     pub safe_to_expose_on_stable: bool,
 }
 
 impl<'tcx> NonConstOp<'tcx> for FnCallUnstable {
     fn status_in_item(&self, _ccx: &ConstCx<'_, 'tcx>) -> Status {
         Status::Unstable {
             gate: self.feature,
+            gate_already_checked: self.feature_enabled,
             safe_to_expose_on_stable: self.safe_to_expose_on_stable,
             is_function_call: true,
         }
     }
 
     fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> Diag<'tcx> {
+        assert!(!self.feature_enabled);
         let mut err = ccx.dcx().create_err(errors::UnstableConstFn {
             span,
             def_path: ccx.tcx.def_path_str(self.def_id),
@@ -376,14 +385,15 @@ impl<'tcx> NonConstOp<'tcx> for IntrinsicNonConst {
 pub(crate) struct IntrinsicUnstable {
     pub name: Symbol,
     pub feature: Symbol,
-    pub const_stable: bool,
+    pub const_stable_indirect: bool,
 }
 
 impl<'tcx> NonConstOp<'tcx> for IntrinsicUnstable {
     fn status_in_item(&self, _ccx: &ConstCx<'_, 'tcx>) -> Status {
         Status::Unstable {
             gate: self.feature,
-            safe_to_expose_on_stable: self.const_stable,
+            gate_already_checked: false,
+            safe_to_expose_on_stable: self.const_stable_indirect,
             // We do *not* want to suggest to mark the intrinsic as `const_stable_indirect`,
             // that's not a trivial change!
             is_function_call: false,
@@ -410,6 +420,7 @@ impl<'tcx> NonConstOp<'tcx> for Coroutine {
         {
             Status::Unstable {
                 gate: sym::const_async_blocks,
+                gate_already_checked: false,
                 safe_to_expose_on_stable: false,
                 is_function_call: false,
             }

tests/ui/consts/min_const_fn/recursive_const_stab_unstable_if_unmarked.rs

@@ -1,6 +1,6 @@
 //@ compile-flags: -Zforce-unstable-if-unmarked
 //@ edition: 2021
-#![feature(const_async_blocks, rustc_attrs, rustc_private)]
+#![feature(const_async_blocks, rustc_attrs)]
 
 pub const fn not_stably_const() {
     // We need to do something const-unstable here.