Skip to content

Commit

Permalink
Rollup merge of #110512 - compiler-errors:fix-elaboration-with-associ…
Browse files Browse the repository at this point in the history
…ated-type-bounds, r=spastorino

Fix elaboration with associated type bounds

When computing a trait's supertrait predicates, do not add any associated type *trait* bounds to that list of supertrait predicates. This is because supertrait predicates are expected to have the same `Self` type as the trait.

For example, given:

```rust
trait Foo: Bar<Assoc: Send>
```

Before, we would compute that the supertrait predicates of `T: Foo` are `T: Bar` and `<T as Bar>::Assoc: Send`. However, the last bound is a trait predicate for a totally different type than `T`, and existing code that uses supertrait bounds such as vtable construction, closure fn signature deduction, etc. all rely on the invariant that we have a list of predicates for self type `T`.

Fixes #76593

The reason for all the extra diagnostic noise is that we're recomputing predicates with a different filter now. These diagnostics should be deduplicated for any end-user though.

---

This does bring up an interesting question -- is the predicate `<T as Bar>::Assoc: Send` an implied bound of `T: Foo`? Because currently the only bounds implied by a (non-alias) trait are its supertraits. I guess I could fix this too, but it would require even more changes, and I'm inclined to punt this question along.
  • Loading branch information
Dylan-DPC authored May 2, 2023
2 parents f379a58 + bec7193 commit be4f9f5
Show file tree
Hide file tree
Showing 26 changed files with 445 additions and 146 deletions.
70 changes: 56 additions & 14 deletions compiler/rustc_hir_analysis/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ use std::slice;
#[derive(Debug)]
pub struct PathSeg(pub DefId, pub usize);

#[derive(Copy, Clone, Debug)]
pub struct OnlySelfBounds(pub bool);

pub trait AstConv<'tcx> {
fn tcx(&self) -> TyCtxt<'tcx>;

Expand Down Expand Up @@ -670,6 +673,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
args: &GenericArgs<'_>,
infer_args: bool,
self_ty: Ty<'tcx>,
only_self_bounds: OnlySelfBounds,
) -> GenericArgCountResult {
let (substs, arg_count) = self.create_substs_for_ast_path(
trait_ref_span,
Expand Down Expand Up @@ -706,6 +710,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
&mut dup_bindings,
binding_span.unwrap_or(binding.span),
constness,
only_self_bounds,
);
// Okay to ignore `Err` because of `ErrorGuaranteed` (see above).
}
Expand Down Expand Up @@ -741,6 +746,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
self_ty: Ty<'tcx>,
bounds: &mut Bounds<'tcx>,
speculative: bool,
only_self_bounds: OnlySelfBounds,
) -> GenericArgCountResult {
let hir_id = trait_ref.hir_ref_id;
let binding_span = None;
Expand All @@ -766,6 +772,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
args,
infer_args,
self_ty,
only_self_bounds,
)
}

Expand All @@ -777,6 +784,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
args: &GenericArgs<'_>,
self_ty: Ty<'tcx>,
bounds: &mut Bounds<'tcx>,
only_self_bounds: OnlySelfBounds,
) {
let binding_span = Some(span);
let constness = ty::BoundConstness::NotConst;
Expand All @@ -799,6 +807,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
args,
infer_args,
self_ty,
only_self_bounds,
);
}

Expand Down Expand Up @@ -947,6 +956,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
ast_bounds: I,
bounds: &mut Bounds<'tcx>,
bound_vars: &'tcx ty::List<ty::BoundVariableKind>,
only_self_bounds: OnlySelfBounds,
) {
for ast_bound in ast_bounds {
match ast_bound {
Expand All @@ -964,11 +974,18 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
param_ty,
bounds,
false,
only_self_bounds,
);
}
&hir::GenericBound::LangItemTrait(lang_item, span, hir_id, args) => {
self.instantiate_lang_item_trait_ref(
lang_item, span, hir_id, args, param_ty, bounds,
lang_item,
span,
hir_id,
args,
param_ty,
bounds,
only_self_bounds,
);
}
hir::GenericBound::Outlives(lifetime) => {
Expand Down Expand Up @@ -1006,8 +1023,19 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
&self,
param_ty: Ty<'tcx>,
ast_bounds: &[hir::GenericBound<'_>],
only_self_bounds: OnlySelfBounds,
) -> Bounds<'tcx> {
self.compute_bounds_inner(param_ty, ast_bounds)
let mut bounds = Bounds::default();
self.add_bounds(
param_ty,
ast_bounds.iter(),
&mut bounds,
ty::List::empty(),
only_self_bounds,
);
debug!(?bounds);

bounds
}

/// Convert the bounds in `ast_bounds` that refer to traits which define an associated type
Expand All @@ -1029,17 +1057,14 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
}
}

self.compute_bounds_inner(param_ty, &result)
}

fn compute_bounds_inner(
&self,
param_ty: Ty<'tcx>,
ast_bounds: &[hir::GenericBound<'_>],
) -> Bounds<'tcx> {
let mut bounds = Bounds::default();

self.add_bounds(param_ty, ast_bounds.iter(), &mut bounds, ty::List::empty());
self.add_bounds(
param_ty,
result.iter(),
&mut bounds,
ty::List::empty(),
OnlySelfBounds(true),
);
debug!(?bounds);

bounds
Expand All @@ -1062,6 +1087,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
dup_bindings: &mut FxHashMap<DefId, Span>,
path_span: Span,
constness: ty::BoundConstness,
only_self_bounds: OnlySelfBounds,
) -> Result<(), ErrorGuaranteed> {
// Given something like `U: SomeTrait<T = X>`, we want to produce a
// predicate like `<U as SomeTrait>::T = X`. This is somewhat
Expand Down Expand Up @@ -1361,8 +1387,20 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
//
// Calling `skip_binder` is okay, because `add_bounds` expects the `param_ty`
// parameter to have a skipped binder.
let param_ty = tcx.mk_alias(ty::Projection, projection_ty.skip_binder());
self.add_bounds(param_ty, ast_bounds.iter(), bounds, projection_ty.bound_vars());
//
// NOTE: If `only_self_bounds` is true, do NOT expand this associated
// type bound into a trait predicate, since we only want to add predicates
// for the `Self` type.
if !only_self_bounds.0 {
let param_ty = tcx.mk_alias(ty::Projection, projection_ty.skip_binder());
self.add_bounds(
param_ty,
ast_bounds.iter(),
bounds,
projection_ty.bound_vars(),
only_self_bounds,
);
}
}
}
Ok(())
Expand Down Expand Up @@ -1403,6 +1441,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
dummy_self,
&mut bounds,
false,
// FIXME: This should be `true`, but we don't really handle
// associated type bounds or type aliases in objects in a way
// that makes this meaningful, I think.
OnlySelfBounds(false),
) {
potential_assoc_types.extend(cur_potential_assoc_types);
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_hir_analysis/src/collect/item_bounds.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::ItemCtxt;
use crate::astconv::AstConv;
use crate::astconv::{AstConv, OnlySelfBounds};
use rustc_hir as hir;
use rustc_infer::traits::util;
use rustc_middle::ty::subst::InternalSubsts;
Expand All @@ -26,7 +26,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);
let mut bounds = icx.astconv().compute_bounds(item_ty, ast_bounds, OnlySelfBounds(false));
// 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 @@ -67,7 +67,7 @@ fn opaque_type_bounds<'tcx>(
) -> &'tcx [(ty::Predicate<'tcx>, Span)] {
ty::print::with_no_queries!({
let icx = ItemCtxt::new(tcx, opaque_def_id);
let mut bounds = icx.astconv().compute_bounds(item_ty, ast_bounds);
let mut bounds = icx.astconv().compute_bounds(item_ty, ast_bounds, OnlySelfBounds(false));
// 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 Down
105 changes: 50 additions & 55 deletions compiler/rustc_hir_analysis/src/collect/predicates_of.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::astconv::AstConv;
use crate::astconv::{AstConv, OnlySelfBounds};
use crate::bounds::Bounds;
use crate::collect::ItemCtxt;
use crate::constrained_generic_params as cgp;
Expand All @@ -14,9 +14,6 @@ use rustc_middle::ty::{GenericPredicates, ToPredicate};
use rustc_span::symbol::{sym, Ident};
use rustc_span::{Span, DUMMY_SP};

#[derive(Debug)]
struct OnlySelfBounds(bool);

/// Returns a list of all type predicates (explicit and implicit) for the definition with
/// ID `def_id`. This includes all predicates returned by `predicates_defined_on`, plus
/// `Self: Trait` predicates for traits.
Expand Down Expand Up @@ -99,8 +96,9 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
| ItemKind::Struct(_, generics)
| ItemKind::Union(_, generics) => generics,

ItemKind::Trait(_, _, generics, ..) | ItemKind::TraitAlias(generics, _) => {
is_trait = Some(ty::TraitRef::identity(tcx, def_id.to_def_id()));
ItemKind::Trait(_, _, generics, self_bounds, ..)
| ItemKind::TraitAlias(generics, self_bounds) => {
is_trait = Some(self_bounds);
generics
}
ItemKind::OpaqueTy(OpaqueTy { generics, .. }) => generics,
Expand All @@ -122,10 +120,14 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen

// Below we'll consider the bounds on the type parameters (including `Self`)
// and the explicit where-clauses, but to get the full set of predicates
// on a trait we need to add in the supertrait bounds and bounds found on
// associated types.
if let Some(_trait_ref) = is_trait {
predicates.extend(tcx.implied_predicates_of(def_id).predicates.iter().cloned());
// on a trait we must also consider the bounds that follow the trait's name,
// like `trait Foo: A + B + C`.
if let Some(self_bounds) = is_trait {
predicates.extend(
icx.astconv()
.compute_bounds(tcx.types.self_param, self_bounds, OnlySelfBounds(false))
.predicates(),
);
}

// In default impls, we can assume that the self type implements
Expand Down Expand Up @@ -225,7 +227,13 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
}

let mut bounds = Bounds::default();
icx.astconv().add_bounds(ty, bound_pred.bounds.iter(), &mut bounds, bound_vars);
icx.astconv().add_bounds(
ty,
bound_pred.bounds.iter(),
&mut bounds,
bound_vars,
OnlySelfBounds(false),
);
predicates.extend(bounds.predicates());
}

Expand Down Expand Up @@ -608,7 +616,7 @@ pub(super) fn implied_predicates_with_filter(
let (superbounds, where_bounds_that_match) = match filter {
PredicateFilter::All => (
// Convert the bounds that follow the colon (or equal in trait aliases)
icx.astconv().compute_bounds(self_param_ty, bounds),
icx.astconv().compute_bounds(self_param_ty, bounds, OnlySelfBounds(false)),
// Also include all where clause bounds
icx.type_parameter_bounds_in_generics(
generics,
Expand All @@ -620,7 +628,7 @@ pub(super) fn implied_predicates_with_filter(
),
PredicateFilter::SelfOnly => (
// Convert the bounds that follow the colon (or equal in trait aliases)
icx.astconv().compute_bounds(self_param_ty, bounds),
icx.astconv().compute_bounds(self_param_ty, bounds, OnlySelfBounds(true)),
// Include where clause bounds for `Self`
icx.type_parameter_bounds_in_generics(
generics,
Expand Down Expand Up @@ -774,32 +782,35 @@ impl<'tcx> ItemCtxt<'tcx> {
only_self_bounds: OnlySelfBounds,
assoc_name: Option<Ident>,
) -> Vec<(ty::Predicate<'tcx>, Span)> {
ast_generics
.predicates
.iter()
.filter_map(|wp| match wp {
hir::WherePredicate::BoundPredicate(bp) => Some(bp),
_ => None,
})
.flat_map(|bp| {
let bt = if bp.is_param_bound(param_def_id.to_def_id()) {
Some(ty)
} else if !only_self_bounds.0 {
Some(self.to_ty(bp.bounded_ty))
} else {
None
};
let bvars = self.tcx.late_bound_vars(bp.hir_id);

bp.bounds.iter().filter_map(move |b| bt.map(|bt| (bt, b, bvars))).filter(
|(_, b, _)| match assoc_name {
Some(assoc_name) => self.bound_defines_assoc_item(b, assoc_name),
None => true,
},
)
})
.flat_map(|(bt, b, bvars)| predicates_from_bound(self, bt, b, bvars))
.collect()
let mut bounds = Bounds::default();

for predicate in ast_generics.predicates {
let hir::WherePredicate::BoundPredicate(predicate) = predicate else {
continue;
};

let bound_ty = if predicate.is_param_bound(param_def_id.to_def_id()) {
ty
} else if !only_self_bounds.0 {
self.to_ty(predicate.bounded_ty)
} else {
continue;
};

let bound_vars = self.tcx.late_bound_vars(predicate.hir_id);
self.astconv().add_bounds(
bound_ty,
predicate.bounds.iter().filter(|bound| {
assoc_name
.map_or(true, |assoc_name| self.bound_defines_assoc_item(bound, assoc_name))
}),
&mut bounds,
bound_vars,
only_self_bounds,
);
}

bounds.predicates().collect()
}

#[instrument(level = "trace", skip(self))]
Expand All @@ -817,19 +828,3 @@ impl<'tcx> ItemCtxt<'tcx> {
}
}
}

/// Converts a specific `GenericBound` from the AST into a set of
/// predicates that apply to the self type. A vector is returned
/// because this can be anywhere from zero predicates (`T: ?Sized` adds no
/// predicates) to one (`T: Foo`) to many (`T: Bar<X = i32>` adds `T: Bar`
/// and `<T as Bar>::X == i32`).
fn predicates_from_bound<'tcx>(
astconv: &dyn AstConv<'tcx>,
param_ty: Ty<'tcx>,
bound: &'tcx hir::GenericBound<'tcx>,
bound_vars: &'tcx ty::List<ty::BoundVariableKind>,
) -> Vec<(ty::Predicate<'tcx>, Span)> {
let mut bounds = Bounds::default();
astconv.add_bounds(param_ty, [bound].into_iter(), &mut bounds, bound_vars);
bounds.predicates().collect()
}
3 changes: 2 additions & 1 deletion compiler/rustc_hir_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode,

use std::ops::Not;

use astconv::AstConv;
use astconv::{AstConv, OnlySelfBounds};
use bounds::Bounds;

fluent_messages! { "../messages.ftl" }
Expand Down Expand Up @@ -531,6 +531,7 @@ pub fn hir_trait_to_predicates<'tcx>(
self_ty,
&mut bounds,
true,
OnlySelfBounds(false),
);

bounds
Expand Down
Loading

0 comments on commit be4f9f5

Please sign in to comment.