From 87c045e2b383ff281458d7b4a2e1c151dc46cbfc Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 22 Oct 2024 17:59:26 +0000 Subject: [PATCH 1/3] Robustify and genericize RTN resolution in RBV --- .../src/collect/resolve_bound_vars.rs | 119 +++++++++++++----- compiler/rustc_hir_analysis/src/lib.rs | 2 + .../all-generics-lookup.rs | 31 +++++ 3 files changed, 119 insertions(+), 33 deletions(-) create mode 100644 tests/ui/associated-type-bounds/all-generics-lookup.rs diff --git a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs index 74729ebe4882f..2887f42c24945 100644 --- a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs +++ b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs @@ -2060,46 +2060,30 @@ 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_in_scope_predicate(path.res).filter_map(|trait_| { BoundVarContext::supertrait_hrtb_vars( self.tcx, trait_.trait_ref.trait_def_id()?, item_segment.ident, ty::AssocKind::Fn, ) - }) - }); + }); + + let one_bound = bounds.next(); + let second_bound = bounds.next(); + + if second_bound.is_some() { + self.tcx + .dcx() + .span_delayed_bug(path.span, "ambiguous resolution for RTN path"); + return; + } + let Some((bound_vars, assoc_item)) = one_bound else { + self.tcx + .dcx() + .span_delayed_bug(path.span, "no resolution for RTN path"); return; }; (bound_vars, assoc_item.def_id, item_segment) @@ -2167,6 +2151,75 @@ 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-ref whose self type + /// corresponds to the expected res. + fn for_each_in_scope_predicate( + &self, + expected_res: Res, + ) -> impl Iterator> + use<'tcx, '_> { + std::iter::from_coroutine( + #[coroutine] + move || { + let mut next_scope = Some(self.scope); + while let Some(current_scope) = next_scope { + next_scope = None; + let hir_id = match *current_scope { + Scope::Binder { s, hir_id, .. } => { + next_scope = Some(s); + hir_id + } + Scope::Body { s, .. } + | Scope::ObjectLifetimeDefault { s, .. } + | Scope::Supertrait { s, .. } + | Scope::TraitRefBoundary { s } + | Scope::LateBoundary { s, .. } => { + next_scope = Some(s); + continue; + } + Scope::Root { opt_parent_item } => { + if let Some(parent_id) = opt_parent_item { + self.tcx.local_def_id_to_hir_id(parent_id) + } else { + continue; + } + } + }; + let node = self.tcx.hir_node(hir_id); + 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; + } + yield pred.bounds; + } + } + // Also consider supertraits for `Self` res... + if let Res::SelfTyParam { trait_: _ } = expected_res + && let hir::Node::Item(item) = node + && let hir::ItemKind::Trait(_, _, _, supertraits, _) = item.kind + { + yield supertraits; + } + } + }, + ) + .flatten() + .filter_map(|pred| match pred { + hir::GenericBound::Trait(poly_trait_ref) => Some(poly_trait_ref), + hir::GenericBound::Outlives(_) | hir::GenericBound::Use(_, _) => None, + }) + .fuse() + } } /// Detects late-bound lifetimes and inserts them into diff --git a/compiler/rustc_hir_analysis/src/lib.rs b/compiler/rustc_hir_analysis/src/lib.rs index 0a26b6776bbd5..58223c868c9b6 100644 --- a/compiler/rustc_hir_analysis/src/lib.rs +++ b/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)] diff --git a/tests/ui/associated-type-bounds/all-generics-lookup.rs b/tests/ui/associated-type-bounds/all-generics-lookup.rs new file mode 100644 index 0000000000000..c5940c14f440c --- /dev/null +++ b/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); + +// 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 Struct +where + T: Trait, +{ + fn test() + where + T::method(..): Send + {} +} + +fn main() { + Struct::<()>::test(); +} From 959801ff2eaf2b3fd76bfc78e4e311be6ce11f9c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 7 Nov 2024 22:41:59 +0000 Subject: [PATCH 2/3] Handle bounds that come from the trait itself --- .../src/collect/resolve_bound_vars.rs | 70 ++++++++++++------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs index 2887f42c24945..8265108513ab0 100644 --- a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs +++ b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs @@ -2061,23 +2061,27 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> { match path.res { Res::Def(DefKind::TyParam, _) | Res::SelfTyParam { trait_: _ } => { let mut bounds = - self.for_each_in_scope_predicate(path.res).filter_map(|trait_| { + 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 one_bound = bounds.next(); - let second_bound = bounds.next(); - if second_bound.is_some() { - self.tcx - .dcx() - .span_delayed_bug(path.span, "ambiguous 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_bound in bounds { + if Some(&second_bound) != one_bound.as_ref() { + self.tcx.dcx().span_delayed_bug( + path.span, + "ambiguous resolution for RTN path", + ); + return; + } } let Some((bound_vars, assoc_item)) = one_bound else { @@ -2086,6 +2090,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> { .span_delayed_bug(path.span, "no 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 @@ -2152,12 +2157,12 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> { self.record_late_bound_vars(item_segment.hir_id, existing_bound_vars_saved); } - /// Walk the generics of the item for a trait-ref whose self type - /// corresponds to the expected res. - fn for_each_in_scope_predicate( + /// 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> + use<'tcx, '_> { + ) -> impl Iterator + use<'tcx, '_> { std::iter::from_coroutine( #[coroutine] move || { @@ -2173,7 +2178,8 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> { | Scope::ObjectLifetimeDefault { s, .. } | Scope::Supertrait { s, .. } | Scope::TraitRefBoundary { s } - | Scope::LateBoundary { s, .. } => { + | Scope::LateBoundary { s, .. } + | Scope::Opaque { s, .. } => { next_scope = Some(s); continue; } @@ -2186,7 +2192,17 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> { } }; let node = self.tcx.hir_node(hir_id); - if let Some(generics) = node.generics() { + // 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; @@ -2200,24 +2216,24 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> { if bounded_path.res != expected_res { continue; } - yield pred.bounds; + 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(_, _) => {} + } + } } } - // Also consider supertraits for `Self` res... - if let Res::SelfTyParam { trait_: _ } = expected_res - && let hir::Node::Item(item) = node - && let hir::ItemKind::Trait(_, _, _, supertraits, _) = item.kind - { - yield supertraits; - } } }, ) - .flatten() - .filter_map(|pred| match pred { - hir::GenericBound::Trait(poly_trait_ref) => Some(poly_trait_ref), - hir::GenericBound::Outlives(_) | hir::GenericBound::Use(_, _) => None, - }) .fuse() } } From 0f5759a00536ac7172090226538bd6870df4b07f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 26 Nov 2024 00:54:59 +0000 Subject: [PATCH 3/3] Address review comments --- .../src/collect/resolve_bound_vars.rs | 142 +++++++++--------- 1 file changed, 74 insertions(+), 68 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs index 8265108513ab0..74f381d266116 100644 --- a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs +++ b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs @@ -2070,12 +2070,19 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> { ) }); - let one_bound = bounds.next(); + 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_bound in bounds { - if Some(&second_bound) != one_bound.as_ref() { + 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", @@ -2084,13 +2091,6 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> { } } - let Some((bound_vars, assoc_item)) = one_bound else { - self.tcx - .dcx() - .span_delayed_bug(path.span, "no 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 @@ -2166,75 +2166,81 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> { std::iter::from_coroutine( #[coroutine] move || { - let mut next_scope = Some(self.scope); - while let Some(current_scope) = next_scope { - next_scope = None; - let hir_id = match *current_scope { - Scope::Binder { s, hir_id, .. } => { - next_scope = Some(s); - hir_id - } - Scope::Body { s, .. } - | Scope::ObjectLifetimeDefault { s, .. } - | Scope::Supertrait { s, .. } - | Scope::TraitRefBoundary { s } - | Scope::LateBoundary { s, .. } - | Scope::Opaque { s, .. } => { - next_scope = Some(s); - continue; - } - Scope::Root { opt_parent_item } => { - if let Some(parent_id) = opt_parent_item { - self.tcx.local_def_id_to_hir_id(parent_id) - } else { - continue; - } + 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, }; - 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; + + 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(_, _) => {} } - 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, + } } }, ) - .fuse() } }