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

Author: RalfJung

compiler/rustc_const_eval/src/check_consts/check.rs

@@ -272,9 +272,18 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
 /// context.
 pubfncheck_op_spanned<O:NonConstOp<'tcx>>(&mutself,op:O,span:Span){
 let gate = match op.status_in_item(self.ccx){
-Status::Unstable{ gate, safe_to_expose_on_stable, is_function_call }
-ifself.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)structFnCallUnstable{
 pubdef_id:DefId,
 pubfeature:Symbol,
+/// If this is true, then the feature is enabled, but we need to still check if it is safe to
+/// expose on stable.
+pubfeature_enabled:bool,
 pubsafe_to_expose_on_stable:bool,
 }
 
 impl<'tcx>NonConstOp<'tcx>forFnCallUnstable{
 fnstatus_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,
 }
 }
 
 fnbuild_error(&self,ccx:&ConstCx<'_,'tcx>,span:Span) -> Diag<'tcx>{
+assert!(!self.feature_enabled);
 letmut 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)structIntrinsicUnstable{
 pubname:Symbol,
 pubfeature:Symbol,
-pubconst_stable:bool,
+pubconst_stable_indirect:bool,
 }
 
 impl<'tcx>NonConstOp<'tcx>forIntrinsicUnstable{
 fnstatus_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)]
 
 pubconstfnnot_stably_const(){
 // We need to do something const-unstable here.