Implement MIR const trait stability checks

This commit is contained in:
Deadbeef 2025-01-25 21:38:15 +08:00
parent 0df0662ee0
commit 4aaf467e26
9 changed files with 232 additions and 120 deletions

View file

@ -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;
@ -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> =
@ -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> {
@ -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 {
@ -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
@ -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

View file

@ -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 {
@ -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

View file

@ -377,11 +377,11 @@ 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
@ -389,24 +389,33 @@ pub(crate) struct FnCallUnstable {
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)]

View file

@ -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 {
@ -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,