Skip to content
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

Implement MIR const trait stability checks #136055

Merged
merged 2 commits into from
Jan 28, 2025
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
3 changes: 2 additions & 1 deletion compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ const_eval_uninhabited_enum_variant_read =
const_eval_uninhabited_enum_variant_written =
writing discriminant of an uninhabited enum variant

const_eval_unmarked_const_fn_exposed = `{$def_path}` cannot be (indirectly) exposed to stable
const_eval_unmarked_const_item_exposed = `{$def_path}` cannot be (indirectly) exposed to stable
.help = either mark the callee as `#[rustc_const_stable_indirect]`, or the caller as `#[rustc_const_unstable]`
const_eval_unmarked_intrinsic_exposed = intrinsic `{$def_path}` cannot be (indirectly) exposed to stable
.help = mark the caller as `#[rustc_const_unstable]`, or mark the intrinsic `#[rustc_intrinsic_const_stable_indirect]` (but this requires team approval)
Expand All @@ -414,6 +414,7 @@ const_eval_unreachable_unwind =

const_eval_unsized_local = unsized locals are not supported
const_eval_unstable_const_fn = `{$def_path}` is not yet stable as a const fn
const_eval_unstable_const_trait = `{$def_path}` is not yet stable as a const trait
const_eval_unstable_in_stable_exposed =
const function that might be (indirectly) exposed to stable cannot use `#[feature({$gate})]`
.is_function_call = mark the callee as `#[rustc_const_stable_indirect]` if it does not itself require any unstable features
Expand Down
168 changes: 87 additions & 81 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::ops::Deref;

use rustc_attr_parsing::{ConstStability, StabilityLevel};
use rustc_errors::{Diag, ErrorGuaranteed};
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir, LangItem};
use rustc_index::bit_set::DenseBitSet;
Expand All @@ -29,7 +30,7 @@ use super::ops::{self, NonConstOp, Status};
use super::qualifs::{self, HasMutInterior, NeedsDrop, NeedsNonConstDrop};
use super::resolver::FlowSensitiveAnalysis;
use super::{ConstCx, Qualif};
use crate::check_consts::is_safe_to_expose_on_stable_const_fn;
use crate::check_consts::is_fn_or_trait_safe_to_expose_on_stable;
use crate::errors;

type QualifResults<'mir, 'tcx, Q> =
Expand Down Expand Up @@ -470,6 +471,88 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
self.tcx.crate_level_attribute_injection_span(self.tcx.local_def_id_to_hir_id(id))
})
}

/// Check the const stability of the given item (fn or trait).
fn check_callee_stability(&mut self, def_id: DefId) {
match self.tcx.lookup_const_stability(def_id) {
Some(ConstStability { level: StabilityLevel::Stable { .. }, .. }) => {
// All good.
}
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.
if self.enforce_recursive_const_stability()
&& !is_fn_or_trait_safe_to_expose_on_stable(self.tcx, def_id)
{
self.dcx().emit_err(errors::UnmarkedConstItemExposed {
span: self.span,
def_path: self.tcx.def_path_str(def_id),
});
}
}
Some(ConstStability {
level: StabilityLevel::Unstable { implied_by: implied_feature, issue, .. },
feature,
..
}) => {
// An unstable const fn/trait with a feature gate.
let callee_safe_to_expose_on_stable =
is_fn_or_trait_safe_to_expose_on_stable(self.tcx, def_id);

// 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
{
return;
}

// We can't use `check_op` to check whether the feature is enabled because
// the logic is a bit different than elsewhere: local functions don't need
// the feature gate, and there might be an "implied" gate that also suffices
// to allow this.
let feature_enabled = def_id.is_local()
|| self.tcx.features().enabled(feature)
|| implied_feature.is_some_and(|f| self.tcx.features().enabled(f))
|| {
// When we're compiling the compiler itself we may pull in
// crates from crates.io, but those crates may depend on other
// crates also pulled in from crates.io. We want to ideally be
// able to compile everything without requiring upstream
// modifications, so in the case that this looks like a
// `rustc_private` crate (e.g., a compiler crate) and we also have
// the `-Z force-unstable-if-unmarked` flag present (we're
// compiling a compiler crate), then let this missing feature
// annotation slide.
// This matches what we do in `eval_stability_allow_unstable` for
// regular stability.
feature == sym::rustc_private
&& issue == NonZero::new(27812)
&& self.tcx.sess.opts.unstable_opts.force_unstable_if_unmarked
};
// 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::CallUnstable {
def_id,
feature,
feature_enabled,
safe_to_expose_on_stable: callee_safe_to_expose_on_stable,
suggestion_span: self.crate_inject_span(),
is_function_call: self.tcx.def_kind(def_id) != DefKind::Trait,
});
}
}
}
}
}

impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
Expand Down Expand Up @@ -716,8 +799,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
span: *fn_span,
call_source,
});
// FIXME(const_trait_impl): do a more fine-grained check whether this
// particular trait can be const-stably called.
self.check_callee_stability(trait_did);
} else {
// Not even a const trait.
self.check_op(ops::FnCallNonConst {
Expand Down Expand Up @@ -793,7 +875,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
// fallback body is safe to expose on stable.
let is_const_stable = intrinsic.const_stable
|| (!intrinsic.must_be_overridden
&& is_safe_to_expose_on_stable_const_fn(tcx, callee));
&& is_fn_or_trait_safe_to_expose_on_stable(tcx, callee));
match tcx.lookup_const_stability(callee) {
None => {
// This doesn't need a separate const-stability check -- const-stability equals
Expand Down Expand Up @@ -842,83 +924,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}

// Finally, stability for regular function calls -- this is the big one.
match tcx.lookup_const_stability(callee) {
Some(ConstStability { level: StabilityLevel::Stable { .. }, .. }) => {
// All good.
}
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.
if self.enforce_recursive_const_stability()
&& !is_safe_to_expose_on_stable_const_fn(tcx, callee)
{
self.dcx().emit_err(errors::UnmarkedConstFnExposed {
span: self.span,
def_path: self.tcx.def_path_str(callee),
});
}
}
Some(ConstStability {
level: StabilityLevel::Unstable { implied_by: implied_feature, issue, .. },
feature,
..
}) => {
// An unstable const fn with a feature gate.
let callee_safe_to_expose_on_stable =
is_safe_to_expose_on_stable_const_fn(tcx, callee);

// 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
{
return;
}

// We can't use `check_op` to check whether the feature is enabled because
// the logic is a bit different than elsewhere: local functions don't need
// the feature gate, and there might be an "implied" gate that also suffices
// to allow this.
let feature_enabled = callee.is_local()
|| tcx.features().enabled(feature)
|| implied_feature.is_some_and(|f| tcx.features().enabled(f))
|| {
// When we're compiling the compiler itself we may pull in
// crates from crates.io, but those crates may depend on other
// crates also pulled in from crates.io. We want to ideally be
// able to compile everything without requiring upstream
// modifications, so in the case that this looks like a
// `rustc_private` crate (e.g., a compiler crate) and we also have
// the `-Z force-unstable-if-unmarked` flag present (we're
// compiling a compiler crate), then let this missing feature
// annotation slide.
// This matches what we do in `eval_stability_allow_unstable` for
// regular stability.
feature == sym::rustc_private
&& issue == NonZero::new(27812)
&& tcx.sess.opts.unstable_opts.force_unstable_if_unmarked
};
// 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,
suggestion_span: self.crate_inject_span(),
});
}
}
}
self.check_callee_stability(callee);
}

// Forbid all `Drop` terminators unless the place being dropped is a local with no
Expand Down
20 changes: 3 additions & 17 deletions compiler/rustc_const_eval/src/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl<'mir, 'tcx> ConstCx<'mir, 'tcx> {
self.const_kind == Some(hir::ConstContext::ConstFn)
&& (self.tcx.features().staged_api()
|| self.tcx.sess.opts.unstable_opts.force_unstable_if_unmarked)
&& is_safe_to_expose_on_stable_const_fn(self.tcx, self.def_id().to_def_id())
&& is_fn_or_trait_safe_to_expose_on_stable(self.tcx, self.def_id().to_def_id())
}

fn is_async(&self) -> bool {
Expand Down Expand Up @@ -84,28 +84,14 @@ pub fn rustc_allow_const_fn_unstable(
attr::rustc_allow_const_fn_unstable(tcx.sess, attrs).any(|name| name == feature_gate)
}

/// Returns `true` if the given `const fn` is "safe to expose on stable".
///
/// Panics if the given `DefId` does not refer to a `const fn`.
/// Returns `true` if the given `def_id` (trait or function) is "safe to expose on stable".
///
/// This is relevant within a `staged_api` crate. Unlike with normal features, the use of unstable
/// const features *recursively* taints the functions that use them. This is to avoid accidentally
/// exposing e.g. the implementation of an unstable const intrinsic on stable. So we partition the
/// world into two functions: those that are safe to expose on stable (and hence may not use
/// unstable features, not even recursively), and those that are not.
pub fn is_safe_to_expose_on_stable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
// A default body in a `#[const_trait]` is not const-stable because const trait fns currently
// cannot be const-stable. These functions can't be called from anything stable, so we shouldn't
// restrict them to only call const-stable functions.
if tcx.is_const_default_method(def_id) {
// FIXME(const_trait_impl): we have to eventually allow some of these if these things can ever be stable.
// They should probably behave like regular `const fn` for that...
return false;
}

// Const-stability is only relevant for `const fn`.
assert!(tcx.is_const_fn(def_id));

pub fn is_fn_or_trait_safe_to_expose_on_stable(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
match tcx.lookup_const_stability(def_id) {
None => {
// In a `staged_api` crate, we do enforce recursive const stability for all unmarked
Expand Down
27 changes: 18 additions & 9 deletions compiler/rustc_const_eval/src/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,36 +377,45 @@ fn build_error_for_const_call<'tcx>(
err
}

/// A call to an `#[unstable]` const fn or `#[rustc_const_unstable]` function.
/// A call to an `#[unstable]` const fn, `#[rustc_const_unstable]` function or trait.
///
/// Contains the name of the feature that would allow the use of this function.
/// Contains the name of the feature that would allow the use of this function/trait.
#[derive(Debug)]
pub(crate) struct FnCallUnstable {
pub(crate) struct CallUnstable {
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,
pub suggestion_span: Option<Span>,
/// true if `def_id` is the function we are calling, false if `def_id` is an unstable trait.
pub is_function_call: bool,
}

impl<'tcx> NonConstOp<'tcx> for FnCallUnstable {
impl<'tcx> NonConstOp<'tcx> for CallUnstable {
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,
is_function_call: self.is_function_call,
}
}

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),
});
let mut err = if self.is_function_call {
ccx.dcx().create_err(errors::UnstableConstFn {
span,
def_path: ccx.tcx.def_path_str(self.def_id),
})
} else {
ccx.dcx().create_err(errors::UnstableConstTrait {
span,
def_path: ccx.tcx.def_path_str(self.def_id),
})
};
// FIXME: make this translatable
let msg = format!("add `#![feature({})]` to the crate attributes to enable", self.feature);
#[allow(rustc::untranslatable_diagnostic)]
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ pub(crate) struct UnstableConstFn {
pub def_path: String,
}

#[derive(Diagnostic)]
#[diag(const_eval_unstable_const_trait)]
pub(crate) struct UnstableConstTrait {
#[primary_span]
pub span: Span,
pub def_path: String,
}

#[derive(Diagnostic)]
#[diag(const_eval_unstable_intrinsic)]
pub(crate) struct UnstableIntrinsic {
Expand All @@ -139,9 +147,9 @@ pub(crate) struct UnstableIntrinsic {
}

#[derive(Diagnostic)]
#[diag(const_eval_unmarked_const_fn_exposed)]
#[diag(const_eval_unmarked_const_item_exposed)]
#[help]
pub(crate) struct UnmarkedConstFnExposed {
pub(crate) struct UnmarkedConstItemExposed {
#[primary_span]
pub span: Span,
pub def_path: String,
Expand Down
1 change: 1 addition & 0 deletions tests/ui/traits/const-traits/staged-api-user-crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ fn non_const_context() {
const fn stable_const_context() {
Unstable::func();
//~^ ERROR cannot call conditionally-const associated function `<staged_api::Unstable as staged_api::MyTrait>::func` in constant functions
//~| ERROR `staged_api::MyTrait` is not yet stable as a const trait
}

fn main() {}
13 changes: 12 additions & 1 deletion tests/ui/traits/const-traits/staged-api-user-crate.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@ LL | Unstable::func();
= help: add `#![feature(const_trait_impl)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

error: aborting due to 1 previous error
error: `staged_api::MyTrait` is not yet stable as a const trait
--> $DIR/staged-api-user-crate.rs:12:5
|
LL | Unstable::func();
| ^^^^^^^^^^^^^^^^
|
help: add `#![feature(unstable)]` to the crate attributes to enable
|
LL + #![feature(unstable)]
|

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0658`.
Loading
Loading