Skip to content

Implement trait const stability #133999

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

Closed
wants to merge 2 commits into from
Closed
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
8 changes: 0 additions & 8 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -411,14 +411,6 @@ 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_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
.unstable_sugg = if the {$is_function_call2 ->
[true] caller
*[false] function
} is not (yet) meant to be exposed to stable, add `#[rustc_const_unstable]` (this is what you probably want to do)
.bypass_sugg = otherwise, as a last resort `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks (this requires team approval)

const_eval_unstable_intrinsic = `{$name}` is not yet stable as a const intrinsic
.help = add `#![feature({$feature})]` to the crate attributes to enable
Expand Down
37 changes: 15 additions & 22 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir, LangItem};
use rustc_index::bit_set::BitSet;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::middle::stability::emit_const_unstable_in_const_stable_exposed_error;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::*;
use rustc_middle::span_bug;
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::{self, Ty, TypeVisitableExt};
use rustc_mir_dataflow::Analysis;
use rustc_mir_dataflow::impls::{MaybeStorageLive, always_storage_live_locals};
use rustc_span::{Span, Symbol, sym};
use rustc_span::{Span, sym};
use rustc_trait_selection::traits::{
Obligation, ObligationCause, ObligationCauseCode, ObligationCtxt,
};
Expand Down Expand Up @@ -287,9 +288,15 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
// if this function wants to be safe-to-expose-on-stable.
if !safe_to_expose_on_stable
&& self.enforce_recursive_const_stability()
&& !super::rustc_allow_const_fn_unstable(self.tcx, self.def_id(), gate)
&& !self.tcx.rustc_allow_const_fn_unstable(self.def_id(), gate)
{
emit_unstable_in_stable_exposed_error(self.ccx, span, gate, is_function_call);
emit_const_unstable_in_const_stable_exposed_error(
self.tcx,
self.def_id(),
span,
gate,
is_function_call,
);
}

return;
Expand Down Expand Up @@ -709,8 +716,11 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
if trait_is_const {
// Trait calls are always conditionally-const.
self.check_op(ops::ConditionallyConstCall { callee, args: fn_args });
// FIXME(const_trait_impl): do a more fine-grained check whether this
// particular trait can be const-stably called.
self.tcx.enforce_trait_const_stability(
trait_did,
*fn_span,
Some(self.def_id()),
);
} else {
// Not even a const trait.
self.check_op(ops::FnCallNonConst {
Expand Down Expand Up @@ -956,20 +966,3 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
fn is_int_bool_float_or_char(ty: Ty<'_>) -> bool {
ty.is_bool() || ty.is_integral() || ty.is_char() || ty.is_floating_point()
}

fn emit_unstable_in_stable_exposed_error(
ccx: &ConstCx<'_, '_>,
span: Span,
gate: Symbol,
is_function_call: bool,
) -> ErrorGuaranteed {
let attr_span = ccx.tcx.def_span(ccx.def_id()).shrink_to_lo();

ccx.dcx().emit_err(errors::UnstableInStableExposed {
gate: gate.to_string(),
span,
attr_span,
is_function_call,
is_function_call2: is_function_call,
})
}
12 changes: 1 addition & 11 deletions compiler/rustc_const_eval/src/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@
//! it finds operations that are invalid in a certain context.

use rustc_errors::DiagCtxtHandle;
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_middle::ty::{self, PolyFnSig, TyCtxt};
use rustc_middle::{bug, mir};
use rustc_span::Symbol;
use {rustc_attr_parsing as attr, rustc_hir as hir};

pub use self::qualifs::Qualif;

Expand Down Expand Up @@ -75,15 +74,6 @@ impl<'mir, 'tcx> ConstCx<'mir, 'tcx> {
}
}

pub fn rustc_allow_const_fn_unstable(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
feature_gate: Symbol,
) -> bool {
let attrs = tcx.hir().attrs(tcx.local_def_id_to_hir_id(def_id));
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`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,14 @@ use tracing::trace;

use super::ConstCx;
use crate::check_consts::check::Checker;
use crate::check_consts::rustc_allow_const_fn_unstable;

/// Returns `true` if we should use the more precise live drop checker that runs after drop
/// elaboration.
pub fn checking_enabled(ccx: &ConstCx<'_, '_>) -> bool {
// Const-stable functions must always use the stable live drop checker...
if ccx.enforce_recursive_const_stability() {
// ...except if they have the feature flag set via `rustc_allow_const_fn_unstable`.
return rustc_allow_const_fn_unstable(
ccx.tcx,
ccx.body.source.def_id().expect_local(),
sym::const_precise_live_drops,
);
return ccx.tcx.rustc_allow_const_fn_unstable(ccx.def_id(), sym::const_precise_live_drops);
}

ccx.tcx.features().const_precise_live_drops()
Expand Down
23 changes: 0 additions & 23 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,6 @@ pub(crate) struct MutablePtrInFinal {
pub kind: InternKind,
}

#[derive(Diagnostic)]
#[diag(const_eval_unstable_in_stable_exposed)]
pub(crate) struct UnstableInStableExposed {
pub gate: String,
#[primary_span]
pub span: Span,
#[help(const_eval_is_function_call)]
pub is_function_call: bool,
/// Need to duplicate the field so that fluent also provides it as a variable...
pub is_function_call2: bool,
#[suggestion(
const_eval_unstable_sugg,
code = "#[rustc_const_unstable(feature = \"...\", issue = \"...\")]\n",
applicability = "has-placeholders"
)]
#[suggestion(
const_eval_bypass_sugg,
code = "#[rustc_allow_const_fn_unstable({gate})]\n",
applicability = "has-placeholders"
)]
pub attr_span: Span,
}

#[derive(Diagnostic)]
#[diag(const_eval_thread_local_access, code = E0625)]
pub(crate) struct ThreadLocalAccessErr {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1640,6 +1640,7 @@ fn check_impl_constness(

let Some(trait_def_id) = hir_trait_ref.trait_def_id() else { return };
if tcx.is_const_trait(trait_def_id) {
tcx.enforce_trait_const_stability(trait_def_id, hir_trait_ref.path.span, None);
return;
}

Expand Down
30 changes: 20 additions & 10 deletions compiler/rustc_hir_analysis/src/collect/predicates_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,14 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
debug!(?predicates);
}

for (clause, span) in predicates.iter().copied() {
// enforce trait const stability for `const Tr` bounds.
let ty::ClauseKind::HostEffect(pred) = clause.kind().skip_binder() else {
continue;
};
tcx.enforce_trait_const_stability(pred.trait_ref.def_id, span, Some(def_id));
}

ty::GenericPredicates {
parent: generics.parent,
predicates: tcx.arena.alloc_from_iter(predicates),
Expand Down Expand Up @@ -1046,16 +1054,18 @@ pub(super) fn const_conditions<'tcx>(
ty::ConstConditions {
parent: has_parent.then(|| tcx.local_parent(def_id).to_def_id()),
predicates: tcx.arena.alloc_from_iter(bounds.clauses().map(|(clause, span)| {
(
clause.kind().map_bound(|clause| match clause {
ty::ClauseKind::HostEffect(ty::HostEffectPredicate {
trait_ref,
constness: ty::BoundConstness::Maybe,
}) => trait_ref,
_ => bug!("converted {clause:?}"),
}),
span,
)
let poly_trait_ref = clause.kind().map_bound(|clause| match clause {
ty::ClauseKind::HostEffect(ty::HostEffectPredicate {
trait_ref,
constness: ty::BoundConstness::Maybe,
}) => trait_ref,
_ => bug!("converted {clause:?}"),
});

// check the const-stability of `Tr` for `~const Tr` bounds
tcx.enforce_trait_const_stability(poly_trait_ref.def_id(), span, Some(def_id));

(poly_trait_ref, span)
})),
}
}
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_middle/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ middle_const_eval_non_int =
middle_const_not_used_in_type_alias =
const parameter `{$ct}` is part of concrete type but not used in parameter list for the `impl Trait` type alias

middle_const_unstable_in_const_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
.unstable_sugg = if the {$is_function_call2 ->
[true] caller
*[false] function
} is not (yet) meant to be exposed to stable, add `#[rustc_const_unstable]` (this is what you probably want to do)
.bypass_sugg = otherwise, as a last resort `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks (this requires team approval)

middle_cycle =
a cycle occurred during layout computation

Expand Down Expand Up @@ -105,6 +114,8 @@ middle_type_length_limit = reached the type-length limit while instantiating `{$
middle_unknown_layout =
the type `{$ty}` has an unknown layout

middle_unstable_const_trait = `{$def_path}` is not yet stable as a const trait

middle_values_too_big =
values of the type `{$ty}` are too big for the target architecture
middle_written_to_path = the full type name has been written to '{$path}'
31 changes: 31 additions & 0 deletions compiler/rustc_middle/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,34 @@ pub struct TypeLengthLimit {
pub path: PathBuf,
pub type_length: usize,
}

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

#[derive(Diagnostic)]
#[diag(middle_const_unstable_in_const_stable_exposed)]
pub struct ConstUnstableInConstStableExposed {
pub gate: String,
#[primary_span]
pub span: Span,
#[help(middle_is_function_call)]
pub is_function_call: bool,
/// Need to duplicate the field so that fluent also provides it as a variable...
pub is_function_call2: bool,
#[suggestion(
middle_unstable_sugg,
code = "#[rustc_const_unstable(feature = \"...\", issue = \"...\")]\n",
applicability = "has-placeholders"
)]
#[suggestion(
middle_bypass_sugg,
code = "#[rustc_allow_const_fn_unstable({gate})]\n",
applicability = "has-placeholders"
)]
pub attr_span: Span,
}
89 changes: 88 additions & 1 deletion compiler/rustc_middle/src/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_attr_parsing::{
use rustc_data_structures::unord::UnordMap;
use rustc_errors::{Applicability, Diag, EmissionGuarantee};
use rustc_feature::GateIssue;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdMap};
use rustc_hir::{self as hir, HirId};
use rustc_macros::{Decodable, Encodable, HashStable, Subdiagnostic};
Expand All @@ -18,7 +19,7 @@ use rustc_session::Session;
use rustc_session::lint::builtin::{DEPRECATED, DEPRECATED_IN_FUTURE, SOFT_UNSTABLE};
use rustc_session::lint::{BuiltinLintDiag, DeprecatedSinceKind, Level, Lint, LintBuffer};
use rustc_session::parse::feature_err_issue;
use rustc_span::{Span, Symbol, sym};
use rustc_span::{ErrorGuaranteed, Span, Symbol, sym};
use tracing::debug;

pub use self::StabilityLevel::*;
Expand Down Expand Up @@ -597,4 +598,90 @@ impl<'tcx> TyCtxt<'tcx> {
pub fn lookup_deprecation(self, id: DefId) -> Option<Deprecation> {
self.lookup_deprecation_entry(id).map(|depr| depr.attr)
}

/// Returns true if `def_id` has an attribute that allows usage of the const unstable feature `feature_gate`.
pub fn rustc_allow_const_fn_unstable(self, def_id: LocalDefId, feature_gate: Symbol) -> bool {
let attrs = self.hir().attrs(self.local_def_id_to_hir_id(def_id));
attr::rustc_allow_const_fn_unstable(self.sess, attrs).any(|name| name == feature_gate)
}

pub fn enforce_trait_const_stability(
self,
trait_def_id: DefId,
span: Span,
parent_def: Option<LocalDefId>,
) {
// This should work pretty much exactly like the function stability logic in
// `compiler/rustc_const_eval/src/check_consts/check.rs`.
// FIXME(const_trait_impl): Find some way to not duplicate that logic.
let Some(ConstStability {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let Some(ConstStability {
// This should work pretty much exactly like the function stability logic in
// `compiler/rustc_const_eval/src/check_consts/check.rs`.
// FIXME: Find some way to not duplicate that logic.
let Some(ConstStability {

level: attr::StabilityLevel::Unstable { implied_by: implied_feature, .. },
feature,
..
}) = self.lookup_const_stability(trait_def_id)
else {
return;
};

let feature_enabled = trait_def_id.is_local()
|| self.features().enabled(feature)
|| implied_feature.is_some_and(|f| self.features().enabled(f));

if !feature_enabled {
let mut diag = self.dcx().create_err(crate::error::UnstableConstTrait {
span,
def_path: self.def_path_str(trait_def_id),
});
self.disabled_nightly_features(&mut diag, None, [(String::new(), feature)]);
diag.emit();
} else if let Some(parent) = parent_def {
Copy link
Member

@RalfJung RalfJung Jan 7, 2025

Choose a reason for hiding this comment

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

I don't understand this -- what is the "parent" here?

The equivalent code for this entire if-else-if in check.rs is

                        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,
                            });
                        }

Here callee_safe_to_expose_on_stable is always false so we should always behave as-if check_op was called.

Copy link
Member Author

Choose a reason for hiding this comment

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

Either a const fn that contains the ~const Tr bound or the callee of a const method.

we should always behave as-if check_op was called.

Yes, this is why this code is derived from check_op_spanned.

Copy link
Member

@RalfJung RalfJung Jan 10, 2025

Choose a reason for hiding this comment

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

Either a const fn that contains the ~const Tr bound or the callee of a const method.

Sorry I don't follow. If you had said "always the const fn that contains the thing we are stability-checking", i.e., the caller, that would make more sense to me. Is this a typo?

The existing const checks have nothing like this, as far as I can see. I have no idea what it means here that we are skipping the check if there is no parent. We should always know the function we are in and that defines whether we are in recursive-const-stability mode.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is why this code is derived from check_op_spanned.

Can you make the structure parallel, i.e. also have a function like that here with essentially the same inputs (without the NonConstOp trait of course). Or ideally share that code rather than duplicate it... maybe we need a compiler/rustc_middle/src/middle/const_stability.rs file for those functions. (stability.rs is already huge.)

// user either has enabled the feature or the unstable feature is allowed inside a macro,
// but if we consider the item we're in to be const stable, we should error as const stable
// items cannot use unstable features.
let is_stable =
matches!(self.def_kind(parent), DefKind::AssocFn | DefKind::Fn | DefKind::Trait)
&& match self.lookup_const_stability(parent) {
None => {
// `const fn`s without const stability attributes in a `staged_api` crate
// are implicitly stable.
self.features().staged_api()
}
Some(stab) => {
// an explicitly stable `const fn`, or an unstable `const fn` that claims to not use any
// other unstably-const features with `const_stable_indirect`
stab.is_const_stable() || stab.const_stable_indirect
}
};
Copy link
Member

Choose a reason for hiding this comment

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

What is this logic? This has to be a duplicate of something in check.rs, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is duplicating enforce_recursive_const_stability.

Copy link
Member

Choose a reason for hiding this comment

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

That logic is fairly critical and definitely should not be duplicated.


// if our parent function is unstable, no need to error
if !is_stable {
return;
}

// if the feature is explicitly allowed, don't error
if self.rustc_allow_const_fn_unstable(parent, feature) {
return;
}

emit_const_unstable_in_const_stable_exposed_error(self, parent, span, feature, false);
}
}
}

pub fn emit_const_unstable_in_const_stable_exposed_error(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
span: Span,
gate: Symbol,
is_function_call: bool,
) -> ErrorGuaranteed {
let attr_span = tcx.def_span(def_id).shrink_to_lo();

tcx.dcx().emit_err(crate::error::ConstUnstableInConstStableExposed {
gate: gate.to_string(),
span,
attr_span,
is_function_call,
is_function_call2: is_function_call,
})
}
6 changes: 4 additions & 2 deletions compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,9 +593,11 @@ impl<'tcx> MissingStabilityAnnotations<'tcx> {
}

fn check_missing_const_stability(&self, def_id: LocalDefId, span: Span) {
let is_const = self.tcx.is_const_fn(def_id.to_def_id());
let is_const = self.tcx.is_const_fn(def_id.to_def_id())
|| (self.tcx.def_kind(def_id.to_def_id()) == DefKind::Trait
&& self.tcx.is_const_trait(def_id.to_def_id()));

// Reachable const fn must have a stability attribute.
// Reachable const fn/trait must have a stability attribute.
if is_const
&& self.effective_visibilities.is_reachable(def_id)
&& self.tcx.lookup_const_stability(def_id).is_none()
Expand Down
Loading
Loading