Skip to content

Ensure constants are WF before calling into CTFE #137972

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

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
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
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
|err, self_ty, trait_id| {
// FIXME(const_trait_impl): Do we need any of this on the non-const codepath?

let trait_ref = TraitRef::from_method(tcx, trait_id, self.args);
let trait_ref = TraitRef::from_assoc_args(tcx, trait_id, self.args);

match self_ty.kind() {
Param(param_ty) => {
Expand Down
44 changes: 44 additions & 0 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,50 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
tcx: TyCtxt<'tcx>,
key: ty::PseudoCanonicalInput<'tcx, GlobalId<'tcx>>,
) -> ::rustc_middle::mir::interpret::EvalToAllocationRawResult<'tcx> {
// Avoid evaluating instances with impossible bounds required to hold as
// this can result in executing code that should never be executed.
//
// We handle this in interpreter internals instead of at callsites (such as
// type system normalization or match exhaustiveness handling) as basically
// *every* place that we invoke CTFE should not be doing so on definitions
// with impossible bounds. Handling it here ensures that we can be certain
// that we haven't missed anywhere.
let instance_def = key.value.instance.def_id();
if tcx.def_kind(instance_def) == DefKind::AnonConst
&& let ty::AnonConstKind::GCEConst = tcx.anon_const_kind(instance_def)
{ // ... do nothing for GCE anon consts as it would cycle
} else if tcx.def_kind(instance_def) == DefKind::AnonConst
&& let ty::AnonConstKind::RepeatExprCount = tcx.anon_const_kind(instance_def)
{
// Instead of erroring when encountering a repeat expr hack const with impossible
// preds we just FCW, as anon consts are unnameable and this code *might* wind up
// supported one day if the anon const is a path expr.
if tcx.instantiate_and_check_impossible_predicates((
instance_def,
tcx.erase_regions(key.value.instance.args),
)) {
if let Some(local_def) = instance_def.as_local() {
tcx.node_span_lint(
rustc_session::lint::builtin::CONST_EVALUATABLE_UNCHECKED,
tcx.local_def_id_to_hir_id(local_def),
tcx.def_span(instance_def),
|lint| {
lint.primary_message(
"cannot use constants which depend on trivially-false where clauses",
);
},
)
} else {
// If the repeat expr count is from some upstream crate then we don't care to
// lint on it as it should have been linted on when compiling the upstream crate.
}
};
} else if tcx
.instantiate_and_check_impossible_predicates((instance_def, key.value.instance.args))
{
return Err(ErrorHandled::TooGeneric(tcx.def_span(instance_def)));
}

// This shouldn't be used for statics, since statics are conceptually places,
// not values -- so what we do here could break pointer identity.
assert!(key.value.promoted.is_some() || !tcx.is_static(key.value.instance.def_id()));
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_const_eval/src/interpret/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
let tcx = *self.tcx;

let trait_def_id = tcx.trait_of_item(def_id).unwrap();
let virtual_trait_ref = ty::TraitRef::from_method(tcx, trait_def_id, virtual_instance.args);
let virtual_trait_ref =
ty::TraitRef::from_assoc_args(tcx, trait_def_id, virtual_instance.args);
let existential_trait_ref = ty::ExistentialTraitRef::erase_self_ty(tcx, virtual_trait_ref);
let concrete_trait_ref = existential_trait_ref.with_self_ty(tcx, dyn_ty);

Expand Down
25 changes: 25 additions & 0 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ pub(crate) fn provide(providers: &mut Providers) {
opaque_ty_origin,
rendered_precise_capturing_args,
const_param_default,
anon_const_kind,
..*providers
};
}
Expand Down Expand Up @@ -1828,3 +1829,27 @@ fn const_param_default<'tcx>(
.lower_const_arg(default_ct, FeedConstTy::Param(def_id.to_def_id(), identity_args));
ty::EarlyBinder::bind(ct)
}

fn anon_const_kind<'tcx>(tcx: TyCtxt<'tcx>, def: LocalDefId) -> ty::AnonConstKind {
let hir_id = tcx.local_def_id_to_hir_id(def);
let const_arg_id = tcx.parent_hir_id(hir_id);
match tcx.hir_node(const_arg_id) {
hir::Node::ConstArg(_) => {
if tcx.features().generic_const_exprs() {
ty::AnonConstKind::GCEConst
} else if tcx.features().min_generic_const_args() {
ty::AnonConstKind::MCGConst
} else if let hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Repeat(_, repeat_count),
..
}) = tcx.hir_node(tcx.parent_hir_id(const_arg_id))
&& repeat_count.hir_id == const_arg_id
{
ty::AnonConstKind::RepeatExprCount
} else {
ty::AnonConstKind::MCGConst
}
}
_ => ty::AnonConstKind::NonTypeSystem,
}
}
71 changes: 30 additions & 41 deletions compiler/rustc_hir_analysis/src/collect/generics_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,27 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics {
}
}

if in_param_ty {
// We do not allow generic parameters in anon consts if we are inside
// of a const parameter type, e.g. `struct Foo<const N: usize, const M: [u8; N]>` is not allowed.
None
} else if tcx.features().generic_const_exprs() {
let parent_node = tcx.parent_hir_node(hir_id);
debug!(?parent_node);
if let Node::Variant(Variant { disr_expr: Some(constant), .. }) = parent_node
&& constant.hir_id == hir_id
{
// enum variant discriminants are not allowed to use any kind of generics
None
} else if let Some(param_id) = tcx.hir_opt_const_param_default_param_def_id(hir_id)
match tcx.anon_const_kind(def_id) {
// Stable: anon consts are not able to use any generic parameters...
ty::AnonConstKind::MCGConst => None,
// we provide generics to repeat expr counts as a backwards compatibility hack. #76200
ty::AnonConstKind::RepeatExprCount => Some(parent_did),

// Even GCE anon const should not be allowed to use generic parameters as it would be
// trivially forward declared uses once desugared. E.g. `const N: [u8; ANON::<N>]`.
//
// We could potentially mirror the hack done for defaults of generic parameters but
// this case just doesn't come up much compared to `const N: u32 = ...`. Long term the
// hack for defaulted parameters should be removed eventually anyway.
ty::AnonConstKind::GCEConst if in_param_ty => None,
// GCE anon consts as a default for a generic parameter should have their provided generics
// "truncated" up to whatever generic parameter this anon const is within the default of.
//
// FIXME(generic_const_exprs): This only handles `const N: usize = /*defid*/` but not type
// parameter defaults, e.g. `T = Foo</*defid*/>`.
ty::AnonConstKind::GCEConst
if let Some(param_id) =
tcx.hir_opt_const_param_default_param_def_id(hir_id) =>
{
// If the def_id we are calling generics_of on is an anon ct default i.e:
//
Expand Down Expand Up @@ -160,36 +168,17 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics {
has_self: generics.has_self,
has_late_bound_regions: generics.has_late_bound_regions,
};
} else {
// HACK(eddyb) this provides the correct generics when
// `feature(generic_const_expressions)` is enabled, so that const expressions
// used with const generics, e.g. `Foo<{N+1}>`, can work at all.
//
// Note that we do not supply the parent generics when using
// `min_const_generics`.
Some(parent_did)
}
} else {
let parent_node = tcx.parent_hir_node(hir_id);
let parent_node = match parent_node {
Node::ConstArg(ca) => tcx.parent_hir_node(ca.hir_id),
_ => parent_node,
};
match parent_node {
// HACK(eddyb) this provides the correct generics for repeat
// expressions' count (i.e. `N` in `[x; N]`), and explicit
// `enum` discriminants (i.e. `D` in `enum Foo { Bar = D }`),
// as they shouldn't be able to cause query cycle errors.
Node::Expr(Expr { kind: ExprKind::Repeat(_, ct), .. })
if ct.anon_const_hir_id() == Some(hir_id) =>
{
Some(parent_did)
}
Node::TyPat(_) => Some(parent_did),
// Field default values inherit the ADT's generics.
Node::Field(_) => Some(parent_did),
_ => None,
ty::AnonConstKind::GCEConst => Some(parent_did),

// Field defaults are allowed to use generic parameters, e.g. `field: u32 = /*defid: N + 1*/`
ty::AnonConstKind::NonTypeSystem
if matches!(tcx.parent_hir_node(hir_id), Node::TyPat(_) | Node::Field(_)) =>
{
Some(parent_did)
}
// Default to no generic parameters for other kinds of anon consts
ty::AnonConstKind::NonTypeSystem => None,
}
}
Node::ConstBlock(_)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ provide! { tcx, def_id, other, cdata,
doc_link_traits_in_scope => {
tcx.arena.alloc_from_iter(cdata.get_doc_link_traits_in_scope(def_id.index))
}
anon_const_kind => { table }
}

pub(in crate::rmeta) fn provide(providers: &mut Providers) {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1562,6 +1562,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
<- tcx.explicit_implied_const_bounds(def_id).skip_binder());
}
}
if let DefKind::AnonConst = def_kind {
record!(self.tables.anon_const_kind[def_id] <- self.tcx.anon_const_kind(def_id));
}
if tcx.impl_method_has_trait_impl_trait_tys(def_id)
&& let Ok(table) = self.tcx.collect_return_position_impl_trait_in_trait_tys(def_id)
{
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ define_tables! {
doc_link_traits_in_scope: Table<DefIndex, LazyArray<DefId>>,
assumed_wf_types_for_rpitit: Table<DefIndex, LazyArray<(Ty<'static>, Span)>>,
opaque_ty_origin: Table<DefIndex, LazyValue<hir::OpaqueTyOrigin<DefId>>>,
anon_const_kind: Table<DefIndex, LazyValue<ty::AnonConstKind>>,
}

#[derive(TyEncodable, TyDecodable)]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/query/erase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ trivial! {
rustc_middle::ty::Asyncness,
rustc_middle::ty::AsyncDestructor,
rustc_middle::ty::BoundVariableKind,
rustc_middle::ty::AnonConstKind,
rustc_middle::ty::DeducedParamAttrs,
rustc_middle::ty::Destructor,
rustc_middle::ty::fast_reject::SimplifiedType,
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2551,6 +2551,12 @@ rustc_queries! {
desc { "estimating codegen size of `{}`", key }
cache_on_disk_if { true }
}

query anon_const_kind(def_id: DefId) -> ty::AnonConstKind {
desc { |tcx| "looking up anon const kind of `{}`", tcx.def_path_str(def_id) }
cache_on_disk_if { def_id.is_local() }
separate_provide_extern
}
}

rustc_query_append! { define_callbacks! }
Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_middle/src/ty/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::borrow::Cow;

use rustc_data_structures::intern::Interned;
use rustc_error_messages::MultiSpan;
use rustc_macros::HashStable;
use rustc_macros::{HashStable, TyDecodable, TyEncodable};
use rustc_type_ir::walk::TypeWalker;
use rustc_type_ir::{self as ir, TypeFlags, WithCachedTypeInfo};

Expand Down Expand Up @@ -259,3 +259,11 @@ impl<'tcx> Const<'tcx> {
TypeWalker::new(self.into())
}
}

#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, TyEncodable, TyDecodable, HashStable)]
pub enum AnonConstKind {
GCEConst,
MCGConst,
RepeatExprCount,
NonTypeSystem,
}
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,10 @@ impl<'tcx> rustc_type_ir::inherent::Safety<TyCtxt<'tcx>> for hir::Safety {
}

impl<'tcx> rustc_type_ir::inherent::Features<TyCtxt<'tcx>> for &'tcx rustc_feature::Features {
fn min_generic_const_args(self) -> bool {
self.min_generic_const_args()
}

fn generic_const_exprs(self) -> bool {
self.generic_const_exprs()
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ pub use self::closure::{
place_to_string_for_capture,
};
pub use self::consts::{
Const, ConstInt, ConstKind, Expr, ExprKind, ScalarInt, UnevaluatedConst, ValTree, ValTreeKind,
Value,
AnonConstKind, Const, ConstInt, ConstKind, Expr, ExprKind, ScalarInt, UnevaluatedConst,
ValTree, ValTreeKind, Value,
};
pub use self::context::{
CtxtInterners, CurrentGcx, DeducedParamAttrs, Feed, FreeRegionInfo, GlobalCtxt, Lift, TyCtxt,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/parameterized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ trivially_parameterized_over_tcx! {
ty::AsyncDestructor,
ty::AssocItemContainer,
ty::Asyncness,
ty::AnonConstKind,
ty::DeducedParamAttrs,
ty::Destructor,
ty::Generics,
Expand Down
18 changes: 15 additions & 3 deletions compiler/rustc_next_trait_solver/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,26 @@ where
) -> QueryResult<I> {
match ct.kind() {
ty::ConstKind::Unevaluated(uv) => {
// `ConstEvaluatable` goals don't really need to exist under `mgca` as we can assume all
// generic const args can be sucessfully evaluated as they have been checked at def site.
//
// The only reason we keep this around is so that wf checking of signatures is guaranteed
// to wind up normalizing constants emitting errors if they are ill formed. The equivalent
// check does not exist for types and results in diverging aliases not being normalized during
// wfck sometimes.
//
// Regardless, the point being that the behaviour of this goal doesn't really matter so we just
// always return `Ok` and evaluate for the CTFE side effect of emitting an error.
if self.cx().features().min_generic_const_args() {
let _ = self.evaluate_const(param_env, uv);
return self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes);
}

// We never return `NoSolution` here as `evaluate_const` emits an
// error itself when failing to evaluate, so emitting an additional fulfillment
// error in that case is unnecessary noise. This may change in the future once
// evaluation failures are allowed to impact selection, e.g. generic const
// expressions in impl headers or `where`-clauses.

// FIXME(generic_const_exprs): Implement handling for generic
// const expressions here.
if let Some(_normalized) = self.evaluate_const(param_env, uv) {
self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ where
self.instantiate_normalizes_to_term(goal, normalized_const.into());
self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
} else {
// FIXME(BoxyUwU) FIXME(min_generic_const_args): I could not figure out how to write a test for this
// as we don't currently support constants in the type system with impossible predicates. It may become
// possible once `min_generic_const_args` has progressed more.
Comment on lines +24 to +26
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe this is possible with GCE? I don't really want to write a load bearing GCE test lol


// In coherence we should never consider an unevaluatable constant to be rigid. It may be failing due to
// impossible predicates (cc #139000 #137972), or a `panic!`, either way we don't want this to influence
// what impls are considered to overlap.
self.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ pub(crate) fn transform_instance<'tcx>(
let upcast_ty = match tcx.trait_of_item(def_id) {
Some(trait_id) => trait_object_ty(
tcx,
ty::Binder::dummy(ty::TraitRef::from_method(tcx, trait_id, instance.args)),
ty::Binder::dummy(ty::TraitRef::from_assoc_args(tcx, trait_id, instance.args)),
),
// drop_in_place won't have a defining trait, skip the upcast
None => instance.args.type_at(0),
Expand Down Expand Up @@ -481,7 +481,7 @@ fn implemented_method<'tcx>(
trait_method = trait_method_bound;
method_id = instance.def_id();
trait_id = tcx.trait_of_item(method_id)?;
trait_ref = ty::EarlyBinder::bind(TraitRef::from_method(tcx, trait_id, instance.args));
trait_ref = ty::EarlyBinder::bind(TraitRef::from_assoc_args(tcx, trait_id, instance.args));
trait_id
} else {
return None;
Expand Down
15 changes: 12 additions & 3 deletions compiler/rustc_trait_selection/src/traits/const_evaluatable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,16 @@ pub fn is_const_evaluatable<'tcx>(
_ => bug!("unexpected constkind in `is_const_evalautable: {unexpanded_ct:?}`"),
}
} else if tcx.features().min_generic_const_args() {
// This is a sanity check to make sure that non-generics consts are checked to
// be evaluatable in case they aren't cchecked elsewhere. This will NOT error
// if the const uses generics, as desired.
// `ConstEvaluatable` goals don't really need to exist under `mgca` as we can assume all
// generic const args can be sucessfully evaluated as they have been checked at def site.
//
// The only reason we keep this around is so that wf checking of signatures is guaranteed
// to wind up normalizing constants emitting errors if they are ill formed. The equivalent
// check does not exist for types and results in diverging aliases not being normalized during
// wfck sometimes.
//
// Regardless, the point being that the behaviour of this goal doesn't really matter so we just
// always return `Ok` and evaluate for the CTFE side effect of emitting an error.
crate::traits::evaluate_const(infcx, unexpanded_ct, param_env);
Ok(())
} else {
Expand Down Expand Up @@ -134,6 +141,8 @@ pub fn is_const_evaluatable<'tcx>(
NotConstEvaluatable::MentionsInfer
} else if uv.has_non_region_param() {
NotConstEvaluatable::MentionsParam
} else if tcx.instantiate_and_check_impossible_predicates((uv.def, uv.args)) {
return Ok(());
} else {
let guar = infcx.dcx().span_delayed_bug(
span,
Expand Down
Loading
Loading