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

This commit is contained in:
Ralf Jung 2024-11-13 11:58:27 +01:00
parent 44f233f251
commit 9760983353
3 changed files with 37 additions and 13 deletions

View File

@ -272,9 +272,18 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
/// context. /// context.
pub fn check_op_spanned<O: NonConstOp<'tcx>>(&mut self, op: O, span: Span) { pub fn check_op_spanned<O: NonConstOp<'tcx>>(&mut self, op: O, span: Span) {
let gate = match op.status_in_item(self.ccx) { let gate = match op.status_in_item(self.ccx) {
Status::Unstable { gate, safe_to_expose_on_stable, is_function_call } Status::Unstable {
if self.tcx.features().enabled(gate) => 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 // Generally this is allowed since the feature gate is enabled -- except
// if this function wants to be safe-to-expose-on-stable. // if this function wants to be safe-to-expose-on-stable.
if !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 { self.check_op(ops::IntrinsicUnstable {
name: intrinsic.name, name: intrinsic.name,
feature, feature,
const_stable: is_const_stable, const_stable_indirect: is_const_stable,
}); });
} }
Some(ConstStability { level: StabilityLevel::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 // We only honor `span.allows_unstable` aka `#[allow_internal_unstable]` if
// the callee is safe to expose, to avoid bypassing recursive stability. // 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) if (self.span.allows_unstable(feature)
|| implied_feature.is_some_and(|f| self.span.allows_unstable(f))) || implied_feature.is_some_and(|f| self.span.allows_unstable(f)))
&& callee_safe_to_expose_on_stable && callee_safe_to_expose_on_stable
@ -830,15 +845,13 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
&& issue == NonZero::new(27812) && issue == NonZero::new(27812)
&& self.tcx.sess.opts.unstable_opts.force_unstable_if_unmarked && 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 // Even if the feature is enabled, we still need check_op to double-check
// recursive const-stability and the callee is not safe-to-expose. In that // this if the callee is not safe to expose on stable.
// case we need `check_op` to do the check. if !feature_enabled || !callee_safe_to_expose_on_stable {
let danger_zone = !callee_safe_to_expose_on_stable
&& self.enforce_recursive_const_stability();
if danger_zone || !feature_enabled {
self.check_op(ops::FnCallUnstable { self.check_op(ops::FnCallUnstable {
def_id: callee, def_id: callee,
feature, feature,
feature_enabled,
safe_to_expose_on_stable: callee_safe_to_expose_on_stable, safe_to_expose_on_stable: callee_safe_to_expose_on_stable,
}); });
} }

View File

@ -28,6 +28,9 @@ pub enum Status {
Unstable { Unstable {
/// The feature that must be enabled to use this operation. /// The feature that must be enabled to use this operation.
gate: Symbol, 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`. /// Whether it is allowed to use this operation from stable `const fn`.
/// This will usually be `false`. /// This will usually be `false`.
safe_to_expose_on_stable: bool, 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. // We use the `const_trait_impl` gate for all conditionally-const calls.
Status::Unstable { Status::Unstable {
gate: sym::const_trait_impl, gate: sym::const_trait_impl,
gate_already_checked: false,
safe_to_expose_on_stable: false, safe_to_expose_on_stable: false,
// We don't want the "mark the callee as `#[rustc_const_stable_indirect]`" hint // We don't want the "mark the callee as `#[rustc_const_stable_indirect]`" hint
is_function_call: false, is_function_call: false,
@ -330,6 +334,9 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
pub(crate) struct FnCallUnstable { pub(crate) struct FnCallUnstable {
pub def_id: DefId, pub def_id: DefId,
pub feature: Symbol, 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, pub safe_to_expose_on_stable: bool,
} }
@ -337,12 +344,14 @@ impl<'tcx> NonConstOp<'tcx> for FnCallUnstable {
fn status_in_item(&self, _ccx: &ConstCx<'_, 'tcx>) -> Status { fn status_in_item(&self, _ccx: &ConstCx<'_, 'tcx>) -> Status {
Status::Unstable { Status::Unstable {
gate: self.feature, gate: self.feature,
gate_already_checked: self.feature_enabled,
safe_to_expose_on_stable: self.safe_to_expose_on_stable, safe_to_expose_on_stable: self.safe_to_expose_on_stable,
is_function_call: true, is_function_call: true,
} }
} }
fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> Diag<'tcx> { fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> Diag<'tcx> {
assert!(!self.feature_enabled);
let mut err = ccx.dcx().create_err(errors::UnstableConstFn { let mut err = ccx.dcx().create_err(errors::UnstableConstFn {
span, span,
def_path: ccx.tcx.def_path_str(self.def_id), 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(crate) struct IntrinsicUnstable {
pub name: Symbol, pub name: Symbol,
pub feature: Symbol, pub feature: Symbol,
pub const_stable: bool, pub const_stable_indirect: bool,
} }
impl<'tcx> NonConstOp<'tcx> for IntrinsicUnstable { impl<'tcx> NonConstOp<'tcx> for IntrinsicUnstable {
fn status_in_item(&self, _ccx: &ConstCx<'_, 'tcx>) -> Status { fn status_in_item(&self, _ccx: &ConstCx<'_, 'tcx>) -> Status {
Status::Unstable { Status::Unstable {
gate: self.feature, 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`, // We do *not* want to suggest to mark the intrinsic as `const_stable_indirect`,
// that's not a trivial change! // that's not a trivial change!
is_function_call: false, is_function_call: false,
@ -410,6 +420,7 @@ impl<'tcx> NonConstOp<'tcx> for Coroutine {
{ {
Status::Unstable { Status::Unstable {
gate: sym::const_async_blocks, gate: sym::const_async_blocks,
gate_already_checked: false,
safe_to_expose_on_stable: false, safe_to_expose_on_stable: false,
is_function_call: false, is_function_call: false,
} }

View File

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