Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 { .. }, .. }) => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
});
}
Expand Down
15 changes: 13 additions & 2 deletions compiler/rustc_const_eval/src/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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,
Expand All @@ -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,
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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)]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this PR, removing rustc_private here would have completely changed the output. So the point of the PR is that even without rustc_private, the output here is as before: suggesting rustc_const_stable_indirect on the callee.


pub const fn not_stably_const() {
// We need to do something const-unstable here.
Expand Down
Loading