Skip to content

Commit

Permalink
Be better at enforcing that const_conditions is only called on const …
Browse files Browse the repository at this point in the history
…items
  • Loading branch information
compiler-errors committed Oct 24, 2024
1 parent 25c9253 commit 0f5a47d
Show file tree
Hide file tree
Showing 12 changed files with 126 additions and 120 deletions.
6 changes: 5 additions & 1 deletion compiler/rustc_hir_analysis/src/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,11 @@ impl<'tcx> Bounds<'tcx> {
host: ty::HostPolarity,
span: Span,
) {
self.clauses.push((bound_trait_ref.to_host_effect_clause(tcx, host), span));
if tcx.is_const_trait(bound_trait_ref.def_id()) {
self.clauses.push((bound_trait_ref.to_host_effect_clause(tcx, host), span));
} else {
tcx.dcx().span_delayed_bug(span, "tried to lower {host:?} bound for non-const trait");
}
}

pub(crate) fn clauses(
Expand Down
21 changes: 11 additions & 10 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@ fn compare_method_predicate_entailment<'tcx>(
);

// FIXME(effects): This should be replaced with a more dedicated method.
let check_const_if_const = tcx.constness(impl_def_id) == hir::Constness::Const;
if check_const_if_const {
let is_conditionally_const = tcx.is_conditionally_const(impl_def_id);
if is_conditionally_const {
// Augment the hybrid param-env with the const conditions
// of the impl header and the trait method.
hybrid_preds.extend(
Expand Down Expand Up @@ -255,7 +255,7 @@ fn compare_method_predicate_entailment<'tcx>(
// This registers the `~const` bounds of the impl method, which we will prove
// using the hybrid param-env that we earlier augmented with the const conditions
// from the impl header and trait method declaration.
if check_const_if_const {
if is_conditionally_const {
for (const_condition, span) in
tcx.const_conditions(impl_m.def_id).instantiate_own_identity()
{
Expand Down Expand Up @@ -1904,9 +1904,8 @@ fn compare_type_predicate_entailment<'tcx>(
let trait_ty_predicates = tcx.predicates_of(trait_ty.def_id);

let impl_ty_own_bounds = impl_ty_predicates.instantiate_own_identity();
let impl_ty_own_const_conditions =
tcx.const_conditions(impl_ty.def_id).instantiate_own_identity();
if impl_ty_own_bounds.len() == 0 && impl_ty_own_const_conditions.len() == 0 {
// If there are no bounds, then there are no const conditions, so no need to check that here.
if impl_ty_own_bounds.len() == 0 {
// Nothing to check.
return Ok(());
}
Expand All @@ -1931,8 +1930,8 @@ fn compare_type_predicate_entailment<'tcx>(
let impl_ty_span = tcx.def_span(impl_ty_def_id);
let normalize_cause = ObligationCause::misc(impl_ty_span, impl_ty_def_id);

let check_const_if_const = tcx.constness(impl_def_id) == hir::Constness::Const;
if check_const_if_const {
let is_conditionally_const = tcx.is_conditionally_const(impl_ty.def_id);
if is_conditionally_const {
// Augment the hybrid param-env with the const conditions
// of the impl header and the trait assoc type.
hybrid_preds.extend(
Expand Down Expand Up @@ -1968,8 +1967,10 @@ fn compare_type_predicate_entailment<'tcx>(
ocx.register_obligation(traits::Obligation::new(tcx, cause, param_env, predicate));
}

if check_const_if_const {
if is_conditionally_const {
// Validate the const conditions of the impl associated type.
let impl_ty_own_const_conditions =
tcx.const_conditions(impl_ty.def_id).instantiate_own_identity();
for (const_condition, span) in impl_ty_own_const_conditions {
let normalize_cause = traits::ObligationCause::misc(span, impl_ty_def_id);
let const_condition = ocx.normalize(&normalize_cause, param_env, const_condition);
Expand Down Expand Up @@ -2081,7 +2082,7 @@ pub(super) fn check_type_bounds<'tcx>(
.collect();

// Only in a const implementation do we need to check that the `~const` item bounds hold.
if tcx.constness(container_id) == hir::Constness::Const {
if tcx.is_conditionally_const(impl_ty_def_id) {
obligations.extend(
tcx.implied_const_bounds(trait_ty.def_id)
.iter_instantiated_copied(tcx, rebased_args)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,7 @@ fn check_impl<'tcx>(
}

// Ensure that the `~const` where clauses of the trait hold for the impl.
if tcx.constness(item.owner_id.def_id) == hir::Constness::Const {
if tcx.is_conditionally_const(item.owner_id.def_id) {
for (bound, _) in
tcx.const_conditions(trait_ref.def_id).instantiate(tcx, trait_ref.args)
{
Expand Down
80 changes: 20 additions & 60 deletions compiler/rustc_hir_analysis/src/collect/predicates_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -888,48 +888,8 @@ pub(super) fn const_conditions<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
) -> ty::ConstConditions<'tcx> {
// This logic is spaghetti, and should be cleaned up. The current methods that are
// defined to deal with constness are very unintuitive.
if tcx.is_const_fn_raw(def_id.to_def_id()) {
// Ok, const fn or method in const trait.
} else {
match tcx.def_kind(def_id) {
DefKind::Trait => {
if !tcx.is_const_trait(def_id.to_def_id()) {
return Default::default();
}
}
DefKind::Impl { .. } => {
// FIXME(effects): Should be using a dedicated function to
// test if this is a const trait impl.
if tcx.constness(def_id) != hir::Constness::Const {
return Default::default();
}
}
DefKind::AssocTy | DefKind::AssocFn => {
let parent_def_id = tcx.local_parent(def_id).to_def_id();
match tcx.associated_item(def_id).container {
ty::AssocItemContainer::TraitContainer => {
if !tcx.is_const_trait(parent_def_id) {
return Default::default();
}
}
ty::AssocItemContainer::ImplContainer => {
// FIXME(effects): Should be using a dedicated function to
// test if this is a const trait impl.
if tcx.constness(parent_def_id) != hir::Constness::Const {
return Default::default();
}
}
}
}
DefKind::Closure | DefKind::OpaqueTy => {
// Closures and RPITs will eventually have const conditions
// for `~const` bounds.
return Default::default();
}
_ => return Default::default(),
}
if !tcx.is_conditionally_const(def_id) {
bug!("const_conditions invoked for item that is not conditionally const: {def_id:?}");
}

let (generics, trait_def_id_and_supertraits, has_parent) = match tcx.hir_node_by_def_id(def_id)
Expand All @@ -940,7 +900,7 @@ pub(super) fn const_conditions<'tcx>(
hir::ItemKind::Trait(_, _, generics, supertraits, _) => {
(generics, Some((item.owner_id.def_id, supertraits)), false)
}
_ => return Default::default(),
_ => bug!("const_conditions called on wrong item: {def_id:?}"),
},
// While associated types are not really const, we do allow them to have `~const`
// bounds and where clauses. `const_conditions` is responsible for gathering
Expand All @@ -950,13 +910,21 @@ pub(super) fn const_conditions<'tcx>(
hir::TraitItemKind::Fn(_, _) | hir::TraitItemKind::Type(_, _) => {
(item.generics, None, true)
}
_ => return Default::default(),
_ => bug!("const_conditions called on wrong item: {def_id:?}"),
},
Node::ImplItem(item) => match item.kind {
hir::ImplItemKind::Fn(_, _) | hir::ImplItemKind::Type(_) => (item.generics, None, true),
_ => return Default::default(),
hir::ImplItemKind::Fn(_, _) | hir::ImplItemKind::Type(_) => {
(item.generics, None, tcx.is_conditionally_const(tcx.local_parent(def_id)))
}
_ => bug!("const_conditions called on wrong item: {def_id:?}"),
},
_ => return Default::default(),
Node::ForeignItem(item) => match item.kind {
hir::ForeignItemKind::Fn(_, _, generics) => (generics, None, false),
_ => bug!("const_conditions called on wrong item: {def_id:?}"),
},
// N.B. Tuple ctors are unconditionally constant.
Node::Ctor(hir::VariantData::Tuple { .. }) => return Default::default(),
_ => bug!("const_conditions called on wrong item: {def_id:?}"),
};

let icx = ItemCtxt::new(tcx, def_id);
Expand Down Expand Up @@ -1017,30 +985,22 @@ pub(super) fn implied_const_bounds<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::PolyTraitRef<'tcx>, Span)]> {
if !tcx.is_conditionally_const(def_id) {
bug!("const_conditions invoked for item that is not conditionally const: {def_id:?}");
}

let bounds = match tcx.hir_node_by_def_id(def_id) {
Node::Item(hir::Item { kind: hir::ItemKind::Trait(..), .. }) => {
if !tcx.is_const_trait(def_id.to_def_id()) {
return ty::EarlyBinder::bind(&[]);
}

implied_predicates_with_filter(
tcx,
def_id.to_def_id(),
PredicateFilter::SelfConstIfConst,
)
}
Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Type(..), .. }) => {
if !tcx.is_const_trait(tcx.local_parent(def_id).to_def_id()) {
return ty::EarlyBinder::bind(&[]);
}

explicit_item_bounds_with_filter(tcx, def_id, PredicateFilter::ConstIfConst)
}
Node::OpaqueTy(..) => {
// We should eventually collect the `~const` bounds on opaques.
return ty::EarlyBinder::bind(&[]);
}
_ => return ty::EarlyBinder::bind(&[]),
_ => bug!("implied_const_bounds called on wrong item: {def_id:?}"),
};

bounds.map_bound(|bounds| {
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_hir_typeck/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -861,12 +861,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// FIXME(effects): Should this be `is_const_fn_raw`? It depends on if we move
// const stability checking here too, I guess.
if self.tcx.is_const_fn(callee_did)
|| self
.tcx
.trait_of_item(callee_did)
.is_some_and(|def_id| self.tcx.is_const_trait(def_id))
{
if self.tcx.is_conditionally_const(callee_did) {
let q = self.tcx.const_conditions(callee_did);
// FIXME(effects): Use this span with a better cause code.
for (cond, _) in q.instantiate(self.tcx, callee_args) {
Expand Down
18 changes: 11 additions & 7 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1423,7 +1423,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
let g = tcx.generics_of(def_id);
record!(self.tables.generics_of[def_id] <- g);
record!(self.tables.explicit_predicates_of[def_id] <- self.tcx.explicit_predicates_of(def_id));
record!(self.tables.const_conditions[def_id] <- self.tcx.const_conditions(def_id));
let inferred_outlives = self.tcx.inferred_outlives_of(def_id);
record_defaulted_array!(self.tables.inferred_outlives_of[def_id] <- inferred_outlives);

Expand All @@ -1434,6 +1433,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
}
}
if tcx.is_conditionally_const(def_id) {
record!(self.tables.const_conditions[def_id] <- self.tcx.const_conditions(def_id));
}
if should_encode_type(tcx, local_id, def_kind) {
record!(self.tables.type_of[def_id] <- self.tcx.type_of(def_id));
}
Expand All @@ -1457,20 +1459,20 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
self.tcx.explicit_super_predicates_of(def_id).skip_binder());
record_defaulted_array!(self.tables.explicit_implied_predicates_of[def_id] <-
self.tcx.explicit_implied_predicates_of(def_id).skip_binder());
record_defaulted_array!(self.tables.implied_const_bounds[def_id]
<- self.tcx.implied_const_bounds(def_id).skip_binder());
let module_children = self.tcx.module_children_local(local_id);
record_array!(self.tables.module_children_non_reexports[def_id] <-
module_children.iter().map(|child| child.res.def_id().index));
if self.tcx.is_const_trait(def_id) {
record_defaulted_array!(self.tables.implied_const_bounds[def_id]
<- self.tcx.implied_const_bounds(def_id).skip_binder());
}
}
if let DefKind::TraitAlias = def_kind {
record!(self.tables.trait_def[def_id] <- self.tcx.trait_def(def_id));
record_defaulted_array!(self.tables.explicit_super_predicates_of[def_id] <-
self.tcx.explicit_super_predicates_of(def_id).skip_binder());
record_defaulted_array!(self.tables.explicit_implied_predicates_of[def_id] <-
self.tcx.explicit_implied_predicates_of(def_id).skip_binder());
record_defaulted_array!(self.tables.implied_const_bounds[def_id]
<- self.tcx.implied_const_bounds(def_id).skip_binder());
}
if let DefKind::Trait | DefKind::Impl { .. } = def_kind {
let associated_item_def_ids = self.tcx.associated_item_def_ids(def_id);
Expand Down Expand Up @@ -1653,8 +1655,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
if let ty::AssocKind::Type = item.kind {
self.encode_explicit_item_bounds(def_id);
self.encode_explicit_item_super_predicates(def_id);
record_defaulted_array!(self.tables.implied_const_bounds[def_id]
<- self.tcx.implied_const_bounds(def_id).skip_binder());
if tcx.is_conditionally_const(def_id) {
record_defaulted_array!(self.tables.implied_const_bounds[def_id]
<- self.tcx.implied_const_bounds(def_id).skip_binder());
}
}
}
AssocItemContainer::ImplContainer => {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
}

fn is_const_impl(self, def_id: DefId) -> bool {
self.constness(def_id) == hir::Constness::Const
self.is_conditionally_const(def_id)
}

fn const_conditions(
Expand Down Expand Up @@ -3140,6 +3140,7 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

// FIXME(effects): Please remove this. It's a footgun.
/// Whether the trait impl is marked const. This does not consider stability or feature gates.
pub fn is_const_trait_impl_raw(self, def_id: DefId) -> bool {
let Some(local_def_id) = def_id.as_local() else { return false };
Expand Down
67 changes: 66 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1999,10 +1999,75 @@ impl<'tcx> TyCtxt<'tcx> {
pub fn is_const_fn_raw(self, def_id: DefId) -> bool {
matches!(
self.def_kind(def_id),
DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(..) | DefKind::Closure
DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(_, CtorKind::Fn) | DefKind::Closure
) && self.constness(def_id) == hir::Constness::Const
}

/// Whether this item is conditionally constant for the purposes of the
/// effects implementation.
///
/// This roughly corresponds to all const functions and other callable
/// items, along with const impls and traits, and associated types within
/// those impls and traits.
pub fn is_conditionally_const(self, def_id: impl Into<DefId>) -> bool {
let def_id: DefId = def_id.into();
match self.def_kind(def_id) {
DefKind::Impl { of_trait: true } => {
self.constness(def_id) == hir::Constness::Const
&& self.is_const_trait(
self.trait_id_of_impl(def_id)
.expect("expected trait for trait implementation"),
)
}
DefKind::Fn | DefKind::Ctor(_, CtorKind::Fn) => {
self.constness(def_id) == hir::Constness::Const
}
DefKind::Trait => self.is_const_trait(def_id),
DefKind::AssocTy | DefKind::AssocFn => {
let parent_def_id = self.parent(def_id);
match self.def_kind(parent_def_id) {
DefKind::Impl { of_trait: false } => {
self.constness(def_id) == hir::Constness::Const
}
DefKind::Impl { of_trait: true } | DefKind::Trait => {
self.is_conditionally_const(parent_def_id)
}
_ => bug!("unexpected parent item of associated item: {parent_def_id:?}"),
}
}
DefKind::Closure | DefKind::OpaqueTy => {
// Closures and RPITs will eventually have const conditions
// for `~const` bounds.
false
}
DefKind::Ctor(_, CtorKind::Const)
| DefKind::Impl { of_trait: false }
| DefKind::Mod
| DefKind::Struct
| DefKind::Union
| DefKind::Enum
| DefKind::Variant
| DefKind::TyAlias
| DefKind::ForeignTy
| DefKind::TraitAlias
| DefKind::TyParam
| DefKind::Const
| DefKind::ConstParam
| DefKind::Static { .. }
| DefKind::AssocConst
| DefKind::Macro(_)
| DefKind::ExternCrate
| DefKind::Use
| DefKind::ForeignMod
| DefKind::AnonConst
| DefKind::InlineConst
| DefKind::Field
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::SyntheticCoroutineBody => false,
}
}

#[inline]
pub fn is_const_trait(self, def_id: DefId) -> bool {
self.trait_def(def_id).constness == hir::Constness::Const
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_ty_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,13 @@ fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> {

// We extend the param-env of our item with the const conditions of the item,
// since we're allowed to assume `~const` bounds hold within the item itself.
predicates.extend(
tcx.const_conditions(def_id)
.instantiate_identity(tcx)
.into_iter()
.map(|(trait_ref, _)| trait_ref.to_host_effect_clause(tcx, ty::HostPolarity::Maybe)),
);
if tcx.is_conditionally_const(def_id) {
predicates.extend(
tcx.const_conditions(def_id).instantiate_identity(tcx).into_iter().map(
|(trait_ref, _)| trait_ref.to_host_effect_clause(tcx, ty::HostPolarity::Maybe),
),
);
}

let local_did = def_id.as_local();

Expand Down
Loading

0 comments on commit 0f5a47d

Please sign in to comment.