Auto merge of rust-lang#132047 - compiler-errors:rbv-rtn-cleanup, r=c…

…jgillot

Robustify and genericize return-type-notation resolution in `resolve_bound_vars`

rust-lang#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:

- rust-lang#109417

compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs

@@ -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
@@ -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

compiler/rustc_hir_analysis/src/lib.rs

@@ -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)]

tests/ui/associated-type-bounds/all-generics-lookup.rs

@@ -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();
+}