Skip to content

Commit

Permalink
Rollup merge of #132047 - compiler-errors:rbv-rtn-cleanup, r=cjgillot
Browse files Browse the repository at this point in the history
Robustify and genericize return-type-notation resolution in `resolve_bound_vars`

#129629 implemented return-type-notation (RTN) in its path form, like `where T::method(..): Bound`. As part of lowering, we must record the late-bound vars for the where clause introduced by the method (namely, its early- and late-bound lifetime arguments, since `where T::method(..)` turns into a higher-ranked where clause over all of the lifetimes according to [RFC 3654](https://rust-lang.github.io/rfcs/3654-return-type-notation.html#converting-to-higher-ranked-trait-bounds)).

However, this logic was only looking at the where clauses of the parent item that the `T::method(..)` bound was written on, and not any parent items. This PR generalizes that logic to look at the parent item (i.e. the outer impl or trait) instead and fixes a (debug only) assertion as an effect.

This logic is also more general and likely easier to adapt to more interesting (though likely very far off) cases like non-lifetime binder `for<T: Trait> T::method(..): Send` bounds.

Tracking:

- #109417
  • Loading branch information
matthiaskrgr authored Dec 1, 2024
2 parents fe4c6e8 + 0f5759a commit 20af878
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 35 deletions.
145 changes: 110 additions & 35 deletions compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2060,48 +2060,37 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
};
match path.res {
Res::Def(DefKind::TyParam, _) | Res::SelfTyParam { trait_: _ } => {
// Get the generics of this type's hir owner. This is *different*
// from the generics of the parameter's definition, since we want
// to be able to resolve an RTN path on a nested body (e.g. method
// inside an impl) using the where clauses on the method.
// FIXME(return_type_notation): Think of some better way of doing this.
let Some(generics) = self.tcx.hir_owner_node(hir_id.owner).generics()
else {
return;
};

// Look for the first bound that contains an associated type that
// matches the segment that we're looking for. We ignore any subsequent
// bounds since we'll be emitting a hard error in HIR lowering, so this
// is purely speculative.
let one_bound = generics.predicates.iter().find_map(|predicate| {
let hir::WherePredicateKind::BoundPredicate(predicate) = predicate.kind
else {
return None;
};
let hir::TyKind::Path(hir::QPath::Resolved(None, bounded_path)) =
predicate.bounded_ty.kind
else {
return None;
};
if bounded_path.res != path.res {
return None;
}
predicate.bounds.iter().find_map(|bound| {
let hir::GenericBound::Trait(trait_) = bound else {
return None;
};
let mut bounds =
self.for_each_trait_bound_on_res(path.res).filter_map(|trait_def_id| {
BoundVarContext::supertrait_hrtb_vars(
self.tcx,
trait_.trait_ref.trait_def_id()?,
trait_def_id,
item_segment.ident,
ty::AssocKind::Fn,
)
})
});
let Some((bound_vars, assoc_item)) = one_bound else {
});

let Some((bound_vars, assoc_item)) = bounds.next() else {
// This will error in HIR lowering.
self.tcx
.dcx()
.span_delayed_bug(path.span, "no resolution for RTN path");
return;
};

// Don't bail if we have identical bounds, which may be collected from
// something like `T: Bound + Bound`, or via elaborating supertraits.
for (second_vars, second_assoc_item) in bounds {
if second_vars != bound_vars || second_assoc_item != assoc_item {
// This will error in HIR lowering.
self.tcx.dcx().span_delayed_bug(
path.span,
"ambiguous resolution for RTN path",
);
return;
}
}

(bound_vars, assoc_item.def_id, item_segment)
}
// If we have a self type alias (in an impl), try to resolve an
Expand Down Expand Up @@ -2167,6 +2156,92 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
existing_bound_vars.extend(bound_vars);
self.record_late_bound_vars(item_segment.hir_id, existing_bound_vars_saved);
}

/// Walk the generics of the item for a trait bound whose self type
/// corresponds to the expected res, and return the trait def id.
fn for_each_trait_bound_on_res(
&self,
expected_res: Res,
) -> impl Iterator<Item = DefId> + use<'tcx, '_> {
std::iter::from_coroutine(
#[coroutine]
move || {
let mut scope = self.scope;
loop {
let hir_id = match *scope {
Scope::Binder { hir_id, .. } => Some(hir_id),
Scope::Root { opt_parent_item: Some(parent_def_id) } => {
Some(self.tcx.local_def_id_to_hir_id(parent_def_id))
}
Scope::Body { .. }
| Scope::ObjectLifetimeDefault { .. }
| Scope::Supertrait { .. }
| Scope::TraitRefBoundary { .. }
| Scope::LateBoundary { .. }
| Scope::Opaque { .. }
| Scope::Root { opt_parent_item: None } => None,
};

if let Some(hir_id) = hir_id {
let node = self.tcx.hir_node(hir_id);
// If this is a `Self` bound in a trait, yield the trait itself.
// Specifically, we don't need to look at any supertraits since
// we already do that in `BoundVarContext::supertrait_hrtb_vars`.
if let Res::SelfTyParam { trait_: _ } = expected_res
&& let hir::Node::Item(item) = node
&& let hir::ItemKind::Trait(..) = item.kind
{
// Yield the trait's def id. Supertraits will be
// elaborated from that.
yield item.owner_id.def_id.to_def_id();
} else if let Some(generics) = node.generics() {
for pred in generics.predicates {
let hir::WherePredicateKind::BoundPredicate(pred) = pred.kind
else {
continue;
};
let hir::TyKind::Path(hir::QPath::Resolved(None, bounded_path)) =
pred.bounded_ty.kind
else {
continue;
};
// Match the expected res.
if bounded_path.res != expected_res {
continue;
}
for pred in pred.bounds {
match pred {
hir::GenericBound::Trait(poly_trait_ref) => {
if let Some(def_id) =
poly_trait_ref.trait_ref.trait_def_id()
{
yield def_id;
}
}
hir::GenericBound::Outlives(_)
| hir::GenericBound::Use(_, _) => {}
}
}
}
}
}

match *scope {
Scope::Binder { s, .. }
| Scope::Body { s, .. }
| Scope::ObjectLifetimeDefault { s, .. }
| Scope::Supertrait { s, .. }
| Scope::TraitRefBoundary { s }
| Scope::LateBoundary { s, .. }
| Scope::Opaque { s, .. } => {
scope = s;
}
Scope::Root { .. } => break,
}
}
},
)
}
}

/// Detects late-bound lifetimes and inserts them into
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ This API is completely unstable and subject to change.
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![doc(rust_logo)]
#![feature(assert_matches)]
#![feature(coroutines)]
#![feature(if_let_guard)]
#![feature(iter_from_coroutine)]
#![feature(iter_intersperse)]
#![feature(let_chains)]
#![feature(never_type)]
Expand Down
31 changes: 31 additions & 0 deletions tests/ui/associated-type-bounds/all-generics-lookup.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//@ check-pass

#![feature(return_type_notation)]

trait Trait {
fn method(&self) -> impl Sized;
}

impl Trait for () {
fn method(&self) -> impl Sized {}
}

struct Struct<T>(T);

// This test used to fail a debug assertion since we weren't resolving the item
// for `T::method(..)` correctly, leading to two bound vars being given the
// index 0. The solution is to look at both generics of `test` and its parent impl.

impl<T> Struct<T>
where
T: Trait,
{
fn test()
where
T::method(..): Send
{}
}

fn main() {
Struct::<()>::test();
}

0 comments on commit 20af878

Please sign in to comment.