Skip to content

Commit

Permalink
Auto merge of rust-lang#121123 - compiler-errors:item-assumptions, r=…
Browse files Browse the repository at this point in the history
…oli-obk

Split an item bounds and an item's super predicates

This is the moral equivalent of rust-lang#107614, but instead for predicates this applies to **item bounds**. This PR splits out the item bounds (i.e. *all* predicates that are assumed to hold for the alias) from the item *super predicates*, which are the subset of item bounds which share the same self type as the alias.

## Why?

Much like rust-lang#107614, there are places in the compiler where we *only* care about super-predicates, and considering predicates that possibly don't have anything to do with the alias is problematic. This includes things like closure signature inference (which is at its core searching for `Self: Fn(..)` style bounds), but also lints like `#[must_use]`, error reporting for aliases, computing type outlives predicates.

Even in cases where considering all of the `item_bounds` doesn't lead to bugs, unnecessarily considering irrelevant bounds does lead to a regression (rust-lang#121121) due to doing extra work in the solver.

## Example 1 - Trait Aliases

This is best explored via an example:

```
type TAIT<T> = impl TraitAlias<T>;

trait TraitAlias<T> = A + B where T: C;
```

The item bounds list for `Tait<T>` will include:
* `Tait<T>: A`
* `Tait<T>: B`
* `T: C`

While `item_super_predicates` query will include just the first two predicates.

Side-note: You may wonder why `T: C` is included in the item bounds for `TAIT`? This is because when we elaborate `TraitAlias<T>`, we will also elaborate all the predicates on the trait.

## Example 2 - Associated Type Bounds

```
type TAIT<T> = impl Iterator<Item: A>;
```

The `item_bounds` list for `TAIT<T>` will include:
* `Tait<T>: Iterator`
* `<Tait<T> as Iterator>::Item: A`

But the `item_super_predicates` will just include the first bound, since that's the only bound that is relevant to the *alias* itself.

## So what

This leads to some diagnostics duplication just like rust-lang#107614, but none of it will be user-facing. We only see it in the UI test suite because we explicitly disable diagnostic deduplication.

Regarding naming, I went with `super_predicates` kind of arbitrarily; this can easily be changed, but I'd consider better names as long as we don't block this PR in perpetuity.
  • Loading branch information
bors committed Mar 21, 2024
2 parents 6e1f7b5 + ce5f8c9 commit 47dd709
Show file tree
Hide file tree
Showing 74 changed files with 757 additions and 174 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
.copied()
.find_map(find_fn_kind_from_did),
ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => tcx
.explicit_item_bounds(def_id)
.explicit_item_super_predicates(def_id)
.iter_instantiated_copied(tcx, args)
.find_map(|(clause, span)| find_fn_kind_from_did((clause, span))),
ty::Closure(_, args) => match args.as_closure().kind() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ fn fn_sig_suggestion<'tcx>(

let asyncness = if tcx.asyncness(assoc.def_id).is_async() {
output = if let ty::Alias(_, alias_ty) = *output.kind() {
tcx.explicit_item_bounds(alias_ty.def_id)
tcx.explicit_item_super_predicates(alias_ty.def_id)
.iter_instantiated_copied(tcx, alias_ty.args)
.find_map(|(bound, _)| bound.as_projection_clause()?.no_bound_vars()?.term.ty())
.unwrap_or_else(|| {
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ pub fn provide(providers: &mut Providers) {
type_alias_is_lazy: type_of::type_alias_is_lazy,
item_bounds: item_bounds::item_bounds,
explicit_item_bounds: item_bounds::explicit_item_bounds,
item_super_predicates: item_bounds::item_super_predicates,
explicit_item_super_predicates: item_bounds::explicit_item_super_predicates,
item_non_self_assumptions: item_bounds::item_non_self_assumptions,
generics_of: generics_of::generics_of,
predicates_of: predicates_of::predicates_of,
predicates_defined_on,
Expand Down Expand Up @@ -633,7 +636,9 @@ fn convert_item(tcx: TyCtxt<'_>, item_id: hir::ItemId) {
tcx.ensure().generics_of(def_id);
tcx.ensure().predicates_of(def_id);
tcx.ensure().explicit_item_bounds(def_id);
tcx.ensure().explicit_item_super_predicates(def_id);
tcx.ensure().item_bounds(def_id);
tcx.ensure().item_super_predicates(def_id);
}

hir::ItemKind::TyAlias(..) => {
Expand Down Expand Up @@ -689,6 +694,7 @@ fn convert_trait_item(tcx: TyCtxt<'_>, trait_item_id: hir::TraitItemId) {

hir::TraitItemKind::Type(_, Some(_)) => {
tcx.ensure().item_bounds(def_id);
tcx.ensure().item_super_predicates(def_id);
tcx.ensure().type_of(def_id);
// Account for `type T = _;`.
let mut visitor = HirPlaceholderCollector::default();
Expand All @@ -698,6 +704,7 @@ fn convert_trait_item(tcx: TyCtxt<'_>, trait_item_id: hir::TraitItemId) {

hir::TraitItemKind::Type(_, None) => {
tcx.ensure().item_bounds(def_id);
tcx.ensure().item_super_predicates(def_id);
// #74612: Visit and try to find bad placeholders
// even if there is no concrete type.
let mut visitor = HirPlaceholderCollector::default();
Expand Down
54 changes: 49 additions & 5 deletions compiler/rustc_hir_analysis/src/collect/item_bounds.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use super::ItemCtxt;
use crate::astconv::{AstConv, PredicateFilter};
use rustc_data_structures::fx::FxIndexSet;
use rustc_hir as hir;
use rustc_infer::traits::util;
use rustc_middle::ty::GenericArgs;
Expand All @@ -19,6 +20,7 @@ fn associated_type_bounds<'tcx>(
assoc_item_def_id: LocalDefId,
ast_bounds: &'tcx [hir::GenericBound<'tcx>],
span: Span,
filter: PredicateFilter,
) -> &'tcx [(ty::Clause<'tcx>, Span)] {
let item_ty = Ty::new_projection(
tcx,
Expand All @@ -27,7 +29,7 @@ fn associated_type_bounds<'tcx>(
);

let icx = ItemCtxt::new(tcx, assoc_item_def_id);
let mut bounds = icx.astconv().compute_bounds(item_ty, ast_bounds, PredicateFilter::All);
let mut bounds = icx.astconv().compute_bounds(item_ty, ast_bounds, filter);
// Associated types are implicitly sized unless a `?Sized` bound is found
icx.astconv().add_implicitly_sized(&mut bounds, item_ty, ast_bounds, None, span);

Expand Down Expand Up @@ -63,10 +65,11 @@ fn opaque_type_bounds<'tcx>(
ast_bounds: &'tcx [hir::GenericBound<'tcx>],
item_ty: Ty<'tcx>,
span: Span,
filter: PredicateFilter,
) -> &'tcx [(ty::Clause<'tcx>, Span)] {
ty::print::with_reduced_queries!({
let icx = ItemCtxt::new(tcx, opaque_def_id);
let mut bounds = icx.astconv().compute_bounds(item_ty, ast_bounds, PredicateFilter::All);
let mut bounds = icx.astconv().compute_bounds(item_ty, ast_bounds, filter);
// Opaque types are implicitly sized unless a `?Sized` bound is found
icx.astconv().add_implicitly_sized(&mut bounds, item_ty, ast_bounds, None, span);
debug!(?bounds);
Expand All @@ -78,6 +81,21 @@ fn opaque_type_bounds<'tcx>(
pub(super) fn explicit_item_bounds(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
) -> ty::EarlyBinder<&'_ [(ty::Clause<'_>, Span)]> {
explicit_item_bounds_with_filter(tcx, def_id, PredicateFilter::All)
}

pub(super) fn explicit_item_super_predicates(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
) -> ty::EarlyBinder<&'_ [(ty::Clause<'_>, Span)]> {
explicit_item_bounds_with_filter(tcx, def_id, PredicateFilter::SelfOnly)
}

pub(super) fn explicit_item_bounds_with_filter(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
filter: PredicateFilter,
) -> ty::EarlyBinder<&'_ [(ty::Clause<'_>, Span)]> {
match tcx.opt_rpitit_info(def_id.to_def_id()) {
// RPITIT's bounds are the same as opaque type bounds, but with
Expand All @@ -95,6 +113,7 @@ pub(super) fn explicit_item_bounds(
ty::GenericArgs::identity_for_item(tcx, def_id),
),
item.span,
filter,
));
}
Some(ty::ImplTraitInTraitData::Impl { .. }) => span_bug!(
Expand All @@ -109,15 +128,15 @@ pub(super) fn explicit_item_bounds(
kind: hir::TraitItemKind::Type(bounds, _),
span,
..
}) => associated_type_bounds(tcx, def_id, bounds, *span),
}) => associated_type_bounds(tcx, def_id, bounds, *span, filter),
hir::Node::Item(hir::Item {
kind: hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds, in_trait: false, .. }),
span,
..
}) => {
let args = GenericArgs::identity_for_item(tcx, def_id);
let item_ty = Ty::new_opaque(tcx, def_id.to_def_id(), args);
opaque_type_bounds(tcx, def_id, bounds, item_ty, *span)
opaque_type_bounds(tcx, def_id, bounds, item_ty, *span, filter)
}
// Since RPITITs are astconv'd as projections in `ast_ty_to_ty`, when we're asking
// for the item bounds of the *opaques* in a trait's default method signature, we
Expand All @@ -135,7 +154,7 @@ pub(super) fn explicit_item_bounds(
let args = GenericArgs::identity_for_item(tcx, def_id);
let item_ty = Ty::new_opaque(tcx, def_id.to_def_id(), args);
tcx.arena.alloc_slice(
&opaque_type_bounds(tcx, def_id, bounds, item_ty, *span)
&opaque_type_bounds(tcx, def_id, bounds, item_ty, *span, filter)
.to_vec()
.fold_with(&mut AssocTyToOpaque { tcx, fn_def_id: fn_def_id.to_def_id() }),
)
Expand All @@ -155,6 +174,31 @@ pub(super) fn item_bounds(
})
}

pub(super) fn item_super_predicates(
tcx: TyCtxt<'_>,
def_id: DefId,
) -> ty::EarlyBinder<&'_ ty::List<ty::Clause<'_>>> {
tcx.explicit_item_super_predicates(def_id).map_bound(|bounds| {
tcx.mk_clauses_from_iter(
util::elaborate(tcx, bounds.iter().map(|&(bound, _span)| bound)).filter_only_self(),
)
})
}

pub(super) fn item_non_self_assumptions(
tcx: TyCtxt<'_>,
def_id: DefId,
) -> ty::EarlyBinder<&'_ ty::List<ty::Clause<'_>>> {
let all_bounds: FxIndexSet<_> = tcx.item_bounds(def_id).skip_binder().iter().collect();
let own_bounds: FxIndexSet<_> =
tcx.item_super_predicates(def_id).skip_binder().iter().collect();
if all_bounds.len() == own_bounds.len() {
ty::EarlyBinder::bind(ty::List::empty())
} else {
ty::EarlyBinder::bind(tcx.mk_clauses_from_iter(all_bounds.difference(&own_bounds).copied()))
}
}

struct AssocTyToOpaque<'tcx> {
tcx: TyCtxt<'tcx>,
fn_def_id: DefId,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
for ty in [first_ty, second_ty] {
for (clause, _) in self
.tcx
.explicit_item_bounds(rpit_def_id)
.explicit_item_super_predicates(rpit_def_id)
.iter_instantiated_copied(self.tcx, args)
{
let pred = clause.kind().rebind(match clause.kind().skip_binder() {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected_ty,
closure_kind,
self.tcx
.explicit_item_bounds(def_id)
.explicit_item_super_predicates(def_id)
.iter_instantiated_copied(self.tcx, args)
.map(|(c, s)| (c.as_predicate(), s)),
),
Expand Down Expand Up @@ -906,7 +906,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => self
.tcx
.explicit_item_bounds(def_id)
.explicit_item_super_predicates(def_id)
.iter_instantiated_copied(self.tcx, args)
.find_map(|(p, s)| get_future_output(p.as_predicate(), s))?,
ty::Error(_) => return Some(ret_ty),
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,10 @@ impl<'tcx> InferCtxt<'tcx> {
let future_trait = self.tcx.require_lang_item(LangItem::Future, None);
let item_def_id = self.tcx.associated_item_def_ids(future_trait)[0];

self.tcx.explicit_item_bounds(def_id).iter_instantiated_copied(self.tcx, args).find_map(
|(predicate, _)| {
self.tcx
.explicit_item_super_predicates(def_id)
.iter_instantiated_copied(self.tcx, args)
.find_map(|(predicate, _)| {
predicate
.kind()
.map_bound(|kind| match kind {
Expand All @@ -417,8 +419,7 @@ impl<'tcx> InferCtxt<'tcx> {
})
.no_bound_vars()
.flatten()
},
)
})
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,17 +299,19 @@ impl<T> Trait<T> for X {
}
(ty::Dynamic(t, _, ty::DynKind::Dyn), ty::Alias(ty::Opaque, alias))
if let Some(def_id) = t.principal_def_id()
&& tcx.explicit_item_bounds(alias.def_id).skip_binder().iter().any(
|(pred, _span)| match pred.kind().skip_binder() {
&& tcx
.explicit_item_super_predicates(alias.def_id)
.skip_binder()
.iter()
.any(|(pred, _span)| match pred.kind().skip_binder() {
ty::ClauseKind::Trait(trait_predicate)
if trait_predicate.polarity
== ty::ImplPolarity::Positive =>
{
trait_predicate.def_id() == def_id
}
_ => false,
},
) =>
}) =>
{
diag.help(format!(
"you can box the `{}` to coerce it to `Box<{}>`, but you'll have to \
Expand Down Expand Up @@ -412,7 +414,7 @@ impl<T> Trait<T> for X {
ty::Alias(..) => values.expected,
_ => values.found,
};
let preds = tcx.explicit_item_bounds(opaque_ty.def_id);
let preds = tcx.explicit_item_super_predicates(opaque_ty.def_id);
for (pred, _span) in preds.skip_binder() {
let ty::ClauseKind::Trait(trait_predicate) = pred.kind().skip_binder()
else {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/outlives/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
alias_ty: ty::AliasTy<'tcx>,
) -> impl Iterator<Item = ty::Region<'tcx>> {
let tcx = self.tcx;
let bounds = tcx.item_bounds(alias_ty.def_id);
let bounds = tcx.item_super_predicates(alias_ty.def_id);
trace!("{:#?}", bounds.skip_binder());
bounds
.iter_instantiated(tcx, alias_ty.args)
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,9 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
ty::Alias(ty::Opaque | ty::Projection, ty::AliasTy { def_id: def, .. }) => {
elaborate(
cx.tcx,
cx.tcx.explicit_item_bounds(def).instantiate_identity_iter_copied(),
cx.tcx
.explicit_item_super_predicates(def)
.instantiate_identity_iter_copied(),
)
// We only care about self bounds for the impl-trait
.filter_only_self()
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,20 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
ty::EarlyBinder::bind(&*output)
}

fn get_explicit_item_super_predicates(
self,
index: DefIndex,
tcx: TyCtxt<'tcx>,
) -> ty::EarlyBinder<&'tcx [(ty::Clause<'tcx>, Span)]> {
let lazy = self.root.tables.explicit_item_super_predicates.get(self, index);
let output = if lazy.is_default() {
&mut []
} else {
tcx.arena.alloc_from_iter(lazy.decode((self, tcx)))
};
ty::EarlyBinder::bind(&*output)
}

fn get_variant(
self,
kind: DefKind,
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 @@ -206,6 +206,7 @@ impl IntoArgs for (CrateNum, SimplifiedType) {

provide! { tcx, def_id, other, cdata,
explicit_item_bounds => { cdata.get_explicit_item_bounds(def_id.index, tcx) }
explicit_item_super_predicates => { cdata.get_explicit_item_super_predicates(def_id.index, tcx) }
explicit_predicates_of => { table }
generics_of => { table }
inferred_outlives_of => { table_defaulted_array }
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1491,6 +1491,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
if let DefKind::OpaqueTy = def_kind {
self.encode_explicit_item_bounds(def_id);
self.encode_explicit_item_super_predicates(def_id);
self.tables
.is_type_alias_impl_trait
.set(def_id.index, self.tcx.is_type_alias_impl_trait(def_id));
Expand Down Expand Up @@ -1599,6 +1600,12 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
record_defaulted_array!(self.tables.explicit_item_bounds[def_id] <- bounds);
}

fn encode_explicit_item_super_predicates(&mut self, def_id: DefId) {
debug!("EncodeContext::encode_explicit_item_super_predicates({:?})", def_id);
let bounds = self.tcx.explicit_item_super_predicates(def_id).skip_binder();
record_defaulted_array!(self.tables.explicit_item_super_predicates[def_id] <- bounds);
}

#[instrument(level = "debug", skip(self))]
fn encode_info_for_assoc_item(&mut self, def_id: DefId) {
let tcx = self.tcx;
Expand All @@ -1611,6 +1618,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
AssocItemContainer::TraitContainer => {
if let ty::AssocKind::Type = item.kind {
self.encode_explicit_item_bounds(def_id);
self.encode_explicit_item_super_predicates(def_id);
}
}
AssocItemContainer::ImplContainer => {
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 @@ -387,6 +387,7 @@ define_tables! {
// corresponding DefPathHash.
def_path_hashes: Table<DefIndex, u64>,
explicit_item_bounds: Table<DefIndex, LazyArray<(ty::Clause<'static>, Span)>>,
explicit_item_super_predicates: Table<DefIndex, LazyArray<(ty::Clause<'static>, Span)>>,
inferred_outlives_of: Table<DefIndex, LazyArray<(ty::Clause<'static>, Span)>>,
inherent_impls: Table<DefIndex, LazyArray<DefIndex>>,
associated_types_for_impl_traits_in_associated_fn: Table<DefIndex, LazyArray<DefId>>,
Expand Down
Loading

0 comments on commit 47dd709

Please sign in to comment.