Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Robustify and genericize return-type-notation resolution in resolve_bound_vars #132047

Merged
merged 3 commits into from
Dec 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my own culture, where should the proper error be emitted?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will error when we probe for the bound in astconv (HIR -> middle).

);
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,
}
}
},
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how hard it would be to reuse the method probing code we have in rustc_hir_typeck.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be pretty difficult to do without inducing cycles, since the lowering code requires lowering trait refs.

}

/// 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();
}
Loading