Skip to content

Commit

Permalink
Rollup merge of rust-lang#129725 - compiler-errors:predicates-of, r=f…
Browse files Browse the repository at this point in the history
…mease

Stop using `ty::GenericPredicates` for non-predicates_of queries

`GenericPredicates` is a struct of several parts: A list of of an item's own predicates, and a parent def id (and some effects related stuff, but ignore that since it's kinda irrelevant). When instantiating these generic predicates, it calls `predicates_of` on the parent and instantiates its predicates, and appends the item's own instantiated predicates too:

https://github.com/rust-lang/rust/blob/acb4e8b6251f1d8da36f08e7a70fa23fc581839e/compiler/rustc_middle/src/ty/generics.rs#L407-L413

Notice how this should result in a recursive set of calls to `predicates_of`... However, `GenericPredicates` is *also* misused by a bunch of *other* queries as a convenient way of passing around a list of predicates. For these queries, we don't ever set the parent def id of the `GenericPredicates`, but if we did, then this would be very easy to mistakenly call `predicates_of` instead of some other intended parent query.

Given that footgun, and the fact that we don't ever even *use* the parent def id in the `GenericPredicates` returned from queries like `explicit_super_predicates_of`, It really has no benefit over just returning `&'tcx [(Clause<'tcx>, Span)]`.

This PR additionally opts to wrap the results of `EarlyBinder`, as we've tended to use that in the return type of these kinds of queries to properly convey that the user has params to deal with, and it also gives a convenient way of iterating over a slice of things after instantiating.
  • Loading branch information
workingjubilee authored Aug 30, 2024
2 parents 1796f7f + 9200452 commit 4e0b6e9
Show file tree
Hide file tree
Showing 20 changed files with 101 additions and 98 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
span: Span,
def_id: LocalDefId,
assoc_name: Ident,
) -> ty::GenericPredicates<'tcx> {
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]> {
self.tcx.at(span).type_param_predicates((self.item_def_id, def_id, assoc_name))
}

Expand Down
59 changes: 29 additions & 30 deletions compiler/rustc_hir_analysis/src/collect/predicates_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,24 +580,24 @@ pub(super) fn explicit_predicates_of<'tcx>(
/// Ensures that the super-predicates of the trait with a `DefId`
/// of `trait_def_id` are lowered and stored. This also ensures that
/// the transitive super-predicates are lowered.
pub(super) fn explicit_super_predicates_of(
tcx: TyCtxt<'_>,
pub(super) fn explicit_super_predicates_of<'tcx>(
tcx: TyCtxt<'tcx>,
trait_def_id: LocalDefId,
) -> ty::GenericPredicates<'_> {
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]> {
implied_predicates_with_filter(tcx, trait_def_id.to_def_id(), PredicateFilter::SelfOnly)
}

pub(super) fn explicit_supertraits_containing_assoc_item(
tcx: TyCtxt<'_>,
pub(super) fn explicit_supertraits_containing_assoc_item<'tcx>(
tcx: TyCtxt<'tcx>,
(trait_def_id, assoc_name): (DefId, Ident),
) -> ty::GenericPredicates<'_> {
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]> {
implied_predicates_with_filter(tcx, trait_def_id, PredicateFilter::SelfThatDefines(assoc_name))
}

pub(super) fn explicit_implied_predicates_of(
tcx: TyCtxt<'_>,
pub(super) fn explicit_implied_predicates_of<'tcx>(
tcx: TyCtxt<'tcx>,
trait_def_id: LocalDefId,
) -> ty::GenericPredicates<'_> {
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]> {
implied_predicates_with_filter(
tcx,
trait_def_id.to_def_id(),
Expand All @@ -612,11 +612,11 @@ pub(super) fn explicit_implied_predicates_of(
/// Ensures that the super-predicates of the trait with a `DefId`
/// of `trait_def_id` are lowered and stored. This also ensures that
/// the transitive super-predicates are lowered.
pub(super) fn implied_predicates_with_filter(
tcx: TyCtxt<'_>,
pub(super) fn implied_predicates_with_filter<'tcx>(
tcx: TyCtxt<'tcx>,
trait_def_id: DefId,
filter: PredicateFilter,
) -> ty::GenericPredicates<'_> {
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]> {
let Some(trait_def_id) = trait_def_id.as_local() else {
// if `assoc_name` is None, then the query should've been redirected to an
// external provider
Expand Down Expand Up @@ -679,20 +679,16 @@ pub(super) fn implied_predicates_with_filter(
_ => {}
}

ty::GenericPredicates {
parent: None,
predicates: implied_bounds,
effects_min_tys: ty::List::empty(),
}
ty::EarlyBinder::bind(implied_bounds)
}

/// Returns the predicates defined on `item_def_id` of the form
/// `X: Foo` where `X` is the type parameter `def_id`.
#[instrument(level = "trace", skip(tcx))]
pub(super) fn type_param_predicates(
tcx: TyCtxt<'_>,
pub(super) fn type_param_predicates<'tcx>(
tcx: TyCtxt<'tcx>,
(item_def_id, def_id, assoc_name): (LocalDefId, LocalDefId, Ident),
) -> ty::GenericPredicates<'_> {
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]> {
use rustc_hir::*;
use rustc_middle::ty::Ty;

Expand All @@ -713,18 +709,20 @@ pub(super) fn type_param_predicates(
tcx.generics_of(item_def_id).parent.map(|def_id| def_id.expect_local())
};

let mut result = parent
.map(|parent| {
let icx = ItemCtxt::new(tcx, parent);
icx.probe_ty_param_bounds(DUMMY_SP, def_id, assoc_name)
})
.unwrap_or_default();
let result = if let Some(parent) = parent {
let icx = ItemCtxt::new(tcx, parent);
icx.probe_ty_param_bounds(DUMMY_SP, def_id, assoc_name)
} else {
ty::EarlyBinder::bind(&[] as &[_])
};
let mut extend = None;

let item_hir_id = tcx.local_def_id_to_hir_id(item_def_id);

let hir_node = tcx.hir_node(item_hir_id);
let Some(hir_generics) = hir_node.generics() else { return result };
let Some(hir_generics) = hir_node.generics() else {
return result;
};
if let Node::Item(item) = hir_node
&& let ItemKind::Trait(..) = item.kind
// Implied `Self: Trait` and supertrait bounds.
Expand All @@ -748,9 +746,10 @@ pub(super) fn type_param_predicates(
_ => false,
}),
);
result.predicates =
tcx.arena.alloc_from_iter(result.predicates.iter().copied().chain(extra_predicates));
result

ty::EarlyBinder::bind(
tcx.arena.alloc_from_iter(result.skip_binder().iter().copied().chain(extra_predicates)),
)
}

impl<'tcx> ItemCtxt<'tcx> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1761,7 +1761,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
break Some((bound_vars.into_iter().collect(), assoc_item));
}
let predicates = tcx.explicit_supertraits_containing_assoc_item((def_id, assoc_name));
let obligations = predicates.predicates.iter().filter_map(|&(pred, _)| {
let obligations = predicates.iter_identity_copied().filter_map(|(pred, _)| {
let bound_predicate = pred.kind();
match bound_predicate.skip_binder() {
ty::ClauseKind::Trait(data) => {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ pub trait HirTyLowerer<'tcx> {
span: Span,
def_id: LocalDefId,
assoc_name: Ident,
) -> ty::GenericPredicates<'tcx>;
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]>;

/// Lower an associated type to a projection.
///
Expand Down Expand Up @@ -831,13 +831,13 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
debug!(?ty_param_def_id, ?assoc_name, ?span);
let tcx = self.tcx();

let predicates = &self.probe_ty_param_bounds(span, ty_param_def_id, assoc_name).predicates;
let predicates = &self.probe_ty_param_bounds(span, ty_param_def_id, assoc_name);
debug!("predicates={:#?}", predicates);

self.probe_single_bound_for_assoc_item(
|| {
let trait_refs = predicates
.iter()
.iter_identity_copied()
.filter_map(|(p, _)| Some(p.as_trait_clause()?.map_bound(|t| t.trait_ref)));
traits::transitive_bounds_that_define_assoc_item(tcx, trait_refs, assoc_name)
},
Expand Down
25 changes: 11 additions & 14 deletions compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,27 +263,24 @@ impl<'tcx> HirTyLowerer<'tcx> for FnCtxt<'_, 'tcx> {
_: Span,
def_id: LocalDefId,
_: Ident,
) -> ty::GenericPredicates<'tcx> {
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]> {
let tcx = self.tcx;
let item_def_id = tcx.hir().ty_param_owner(def_id);
let generics = tcx.generics_of(item_def_id);
let index = generics.param_def_id_to_index[&def_id.to_def_id()];
// HACK(eddyb) should get the original `Span`.
let span = tcx.def_span(def_id);
ty::GenericPredicates {
parent: None,
predicates: tcx.arena.alloc_from_iter(
self.param_env.caller_bounds().iter().filter_map(|predicate| {
match predicate.kind().skip_binder() {
ty::ClauseKind::Trait(data) if data.self_ty().is_param(index) => {
Some((predicate, span))
}
_ => None,

ty::EarlyBinder::bind(tcx.arena.alloc_from_iter(
self.param_env.caller_bounds().iter().filter_map(|predicate| {
match predicate.kind().skip_binder() {
ty::ClauseKind::Trait(data) if data.self_ty().is_param(index) => {
Some((predicate, span))
}
}),
),
effects_min_tys: ty::List::empty(),
}
_ => None,
}
}),
))
}

fn lower_assoc_ty(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub fn transitive_bounds_that_define_assoc_item<'tcx>(

stack.extend(
tcx.explicit_supertraits_containing_assoc_item((trait_ref.def_id(), assoc_name))
.instantiate_own_identity()
.iter_identity_copied()
.map(|(clause, _)| clause.instantiate_supertrait(tcx, trait_ref))
.filter_map(|clause| clause.as_trait_clause())
// FIXME: Negative supertraits are elaborated here lol
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_lint/src/multiple_supertrait_upcastable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleSupertraitUpcastable {
let direct_super_traits_iter = cx
.tcx
.explicit_super_predicates_of(def_id)
.predicates
.into_iter()
.iter_identity_copied()
.filter_map(|(pred, _)| pred.as_trait_clause());
if direct_super_traits_iter.count() > 1 {
cx.emit_span_lint(
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,24 @@ impl<'a, 'tcx, T: Copy + Decodable<DecodeContext<'a, 'tcx>>> ProcessQueryValue<'
}
}

impl<'a, 'tcx, T: Copy + Decodable<DecodeContext<'a, 'tcx>>>
ProcessQueryValue<'tcx, ty::EarlyBinder<'tcx, &'tcx [T]>>
for Option<DecodeIterator<'a, 'tcx, T>>
{
#[inline(always)]
fn process_decoded(
self,
tcx: TyCtxt<'tcx>,
_err: impl Fn() -> !,
) -> ty::EarlyBinder<'tcx, &'tcx [T]> {
ty::EarlyBinder::bind(if let Some(iter) = self {
tcx.arena.alloc_from_iter(iter)
} else {
&[]
})
}
}

impl<'a, 'tcx, T: Copy + Decodable<DecodeContext<'a, 'tcx>>>
ProcessQueryValue<'tcx, Option<&'tcx [T]>> for Option<DecodeIterator<'a, 'tcx, T>>
{
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1446,17 +1446,21 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
if let DefKind::Trait = def_kind {
record!(self.tables.trait_def[def_id] <- self.tcx.trait_def(def_id));
record!(self.tables.explicit_super_predicates_of[def_id] <- self.tcx.explicit_super_predicates_of(def_id));
record!(self.tables.explicit_implied_predicates_of[def_id] <- self.tcx.explicit_implied_predicates_of(def_id));
record_array!(self.tables.explicit_super_predicates_of[def_id] <-
self.tcx.explicit_super_predicates_of(def_id).skip_binder());
record_array!(self.tables.explicit_implied_predicates_of[def_id] <-
self.tcx.explicit_implied_predicates_of(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 let DefKind::TraitAlias = def_kind {
record!(self.tables.trait_def[def_id] <- self.tcx.trait_def(def_id));
record!(self.tables.explicit_super_predicates_of[def_id] <- self.tcx.explicit_super_predicates_of(def_id));
record!(self.tables.explicit_implied_predicates_of[def_id] <- self.tcx.explicit_implied_predicates_of(def_id));
record_array!(self.tables.explicit_super_predicates_of[def_id] <-
self.tcx.explicit_super_predicates_of(def_id).skip_binder());
record_array!(self.tables.explicit_implied_predicates_of[def_id] <-
self.tcx.explicit_implied_predicates_of(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
4 changes: 2 additions & 2 deletions compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,10 +419,10 @@ define_tables! {
lookup_deprecation_entry: Table<DefIndex, LazyValue<attr::Deprecation>>,
explicit_predicates_of: Table<DefIndex, LazyValue<ty::GenericPredicates<'static>>>,
generics_of: Table<DefIndex, LazyValue<ty::Generics>>,
explicit_super_predicates_of: Table<DefIndex, LazyValue<ty::GenericPredicates<'static>>>,
explicit_super_predicates_of: Table<DefIndex, LazyArray<(ty::Clause<'static>, Span)>>,
// As an optimization, we only store this for trait aliases,
// since it's identical to explicit_super_predicates_of for traits.
explicit_implied_predicates_of: Table<DefIndex, LazyValue<ty::GenericPredicates<'static>>>,
explicit_implied_predicates_of: Table<DefIndex, LazyArray<(ty::Clause<'static>, Span)>>,
type_of: Table<DefIndex, LazyValue<ty::EarlyBinder<'static, Ty<'static>>>>,
variances_of: Table<DefIndex, LazyArray<ty::Variance>>,
fn_sig: Table<DefIndex, LazyValue<ty::EarlyBinder<'static, ty::PolyFnSig<'static>>>>,
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ rustc_queries! {
/// is a subset of the full list of predicates. We store these in a separate map
/// because we must evaluate them even during type conversion, often before the full
/// predicates are available (note that super-predicates must not be cyclic).
query explicit_super_predicates_of(key: DefId) -> ty::GenericPredicates<'tcx> {
query explicit_super_predicates_of(key: DefId) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]> {
desc { |tcx| "computing the super predicates of `{}`", tcx.def_path_str(key) }
cache_on_disk_if { key.is_local() }
separate_provide_extern
Expand All @@ -662,7 +662,7 @@ rustc_queries! {
/// of the trait. For regular traits, this includes all super-predicates and their
/// associated type bounds. For trait aliases, currently, this includes all of the
/// predicates of the trait alias.
query explicit_implied_predicates_of(key: DefId) -> ty::GenericPredicates<'tcx> {
query explicit_implied_predicates_of(key: DefId) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]> {
desc { |tcx| "computing the implied predicates of `{}`", tcx.def_path_str(key) }
cache_on_disk_if { key.is_local() }
separate_provide_extern
Expand All @@ -671,7 +671,9 @@ rustc_queries! {
/// The Ident is the name of an associated type.The query returns only the subset
/// of supertraits that define the given associated type. This is used to avoid
/// cycles in resolving type-dependent associated item paths like `T::Item`.
query explicit_supertraits_containing_assoc_item(key: (DefId, rustc_span::symbol::Ident)) -> ty::GenericPredicates<'tcx> {
query explicit_supertraits_containing_assoc_item(
key: (DefId, rustc_span::symbol::Ident)
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]> {
desc { |tcx| "computing the super traits of `{}` with associated type name `{}`",
tcx.def_path_str(key.0),
key.1
Expand All @@ -680,7 +682,9 @@ rustc_queries! {

/// To avoid cycles within the predicates of a single item we compute
/// per-type-parameter predicates for resolving `T::AssocTy`.
query type_param_predicates(key: (LocalDefId, LocalDefId, rustc_span::symbol::Ident)) -> ty::GenericPredicates<'tcx> {
query type_param_predicates(
key: (LocalDefId, LocalDefId, rustc_span::symbol::Ident)
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]> {
desc { |tcx| "computing the bounds for type parameter `{}`", tcx.hir().ty_param_name(key.1) }
}

Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,16 +349,14 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
self,
def_id: DefId,
) -> ty::EarlyBinder<'tcx, impl IntoIterator<Item = (ty::Clause<'tcx>, Span)>> {
ty::EarlyBinder::bind(self.explicit_super_predicates_of(def_id).instantiate_identity(self))
self.explicit_super_predicates_of(def_id).map_bound(|preds| preds.into_iter().copied())
}

fn explicit_implied_predicates_of(
self,
def_id: DefId,
) -> ty::EarlyBinder<'tcx, impl IntoIterator<Item = (ty::Clause<'tcx>, Span)>> {
ty::EarlyBinder::bind(
self.explicit_implied_predicates_of(def_id).instantiate_identity(self),
)
self.explicit_implied_predicates_of(def_id).map_bound(|preds| preds.into_iter().copied())
}

fn has_target_features(self, def_id: DefId) -> bool {
Expand Down
10 changes: 4 additions & 6 deletions compiler/rustc_trait_selection/src/traits/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,11 @@ fn predicates_reference_self(
) -> SmallVec<[Span; 1]> {
let trait_ref = ty::Binder::dummy(ty::TraitRef::identity(tcx, trait_def_id));
let predicates = if supertraits_only {
tcx.explicit_super_predicates_of(trait_def_id)
tcx.explicit_super_predicates_of(trait_def_id).skip_binder()
} else {
tcx.predicates_of(trait_def_id)
tcx.predicates_of(trait_def_id).predicates
};
predicates
.predicates
.iter()
.map(|&(predicate, sp)| (predicate.instantiate_supertrait(tcx, trait_ref), sp))
.filter_map(|(clause, sp)| {
Expand Down Expand Up @@ -266,9 +265,8 @@ fn super_predicates_have_non_lifetime_binders(
return SmallVec::new();
}
tcx.explicit_super_predicates_of(trait_def_id)
.predicates
.iter()
.filter_map(|(pred, span)| pred.has_non_region_bound_vars().then_some(*span))
.iter_identity_copied()
.filter_map(|(pred, span)| pred.has_non_region_bound_vars().then_some(span))
.collect()
}

Expand Down
12 changes: 5 additions & 7 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,21 +600,19 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

// Check supertraits hold. This is so that their associated type bounds
// will be checked in the code below.
for super_trait in tcx
for (supertrait, _) in tcx
.explicit_super_predicates_of(trait_predicate.def_id())
.instantiate(tcx, trait_predicate.trait_ref.args)
.predicates
.into_iter()
.iter_instantiated_copied(tcx, trait_predicate.trait_ref.args)
{
let normalized_super_trait = normalize_with_depth_to(
let normalized_supertrait = normalize_with_depth_to(
self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth + 1,
super_trait,
supertrait,
&mut nested,
);
nested.push(obligation.with(tcx, normalized_super_trait));
nested.push(obligation.with(tcx, normalized_supertrait));
}

let assoc_types: Vec<_> = tcx
Expand Down
Loading

0 comments on commit 4e0b6e9

Please sign in to comment.