Skip to content

Commit

Permalink
Implement MIR const trait stability checks
Browse files Browse the repository at this point in the history
  • Loading branch information
fee1-dead committed Jan 27, 2025
1 parent f940188 commit 085a10a
Show file tree
Hide file tree
Showing 10 changed files with 234 additions and 122 deletions.
5 changes: 3 additions & 2 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,8 @@ 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
.help = either mark the callee as `#[rustc_const_stable_indirect]`, or the caller as `#[rustc_const_unstable]`
const_eval_unmarked_const_item_exposed = `{$def_path}` cannot be (indirectly) exposed to stable
.help = either mark the item 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 unsafe 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 @@ -694,6 +695,88 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}
};

let check_stability = |this: &mut Self, def_id| {
match 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 this.enforce_recursive_const_stability()
&& !is_fn_or_trait_safe_to_expose_on_stable(tcx, def_id)
{
this.dcx().emit_err(errors::UnmarkedConstItemExposed {
span: this.span,
def_path: this.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(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 (this.span.allows_unstable(feature)
|| implied_feature.is_some_and(|f| this.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()
|| 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 {
this.check_op(ops::CallUnstable {
def_id,
feature,
feature_enabled,
safe_to_expose_on_stable: callee_safe_to_expose_on_stable,
suggestion_span: this.crate_inject_span(),
is_function_call: tcx.def_kind(def_id) != DefKind::Trait,
});
}
}
}
};

let has_const_conditions =
self.revalidate_conditional_constness(callee, fn_args, *fn_span);

Expand All @@ -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.
check_stability(self, 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(),
});
}
}
}
check_stability(self, 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
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ error: `just_a_fn` cannot be (indirectly) exposed to stable
LL | unmarked_const_fn_crate::just_a_fn();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: either mark the callee as `#[rustc_const_stable_indirect]`, or the caller as `#[rustc_const_unstable]`
= help: either mark the item as `#[rustc_const_stable_indirect]`, or the caller as `#[rustc_const_unstable]`

error: aborting due to 2 previous errors

Loading

0 comments on commit 085a10a

Please sign in to comment.