Add supertrait item shadowing for type-level path resolution#152225
Add supertrait item shadowing for type-level path resolution#152225Amanieu wants to merge 1 commit intorust-lang:mainfrom
Conversation
|
HIR ty lowering was modified cc @fmease |
rustbot
added
S-waiting-on-review
|
r? @madsmtm rustbot has assigned @madsmtm. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| println!("{}", size_of::<T::T>()); | ||
| } | ||
|
|
||
| fn generic2<T: C<T = i16>>() { |
There was a problem hiding this comment.
What about T: B<T = i16> and T: A<T = i8> - do those resolve to B::T and A::T respectively?
Also, please use a different name for the generic and the associated type, it's a bit confusing as-is.
There was a problem hiding this comment.
I've cleaned up the example and added tests showing how B<T = i16> and A<T = i8> are resolved.
|
r? types |
rustbot
added
the
T-types
traviscross
added
the
I-lang-radar
Wait, I'm not sure if that should've unassigned me or not? In any case, if you also need a compiler reviewer, I'm the wrong one for the job, so I'm gonna unassign myself. |
Amanieu
force-pushed
the
type-supertrait-shadowing
branch
from
1cde4c7 to
5a9ef14
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
There was a problem hiding this comment.
In general, I think this needs some more tests. I've already commented about adding the testing attr, but ideally we should go through the existing tests/ui/methods/supertrait-shadowing tests (which in general are pretty thorough) and write the equivalent test for associated types and consts. These cover cases like multiple ancestors, unsatisfied trait bounds, scoping, and (in a PR I'm about to make) stability.
I also would like to see test that show that <U as A>::T works just fine, even if you have U: B.
| use std::mem::size_of; | ||
|
|
||
| trait A { | ||
| type T; |
There was a problem hiding this comment.
Please use a different name for the associated type to name lexically clash with the param.
| println!("{}", size_of::<U::T>()); | ||
| } | ||
|
|
||
| fn generic3<U: B<T = i16>>() { |
There was a problem hiding this comment.
What is missing here is a test of U: B<T = i8> and U: B, just to show the former is not callable (because of the blanket impl), and the latter still uses <U as B>::T. (Similar for C below)
| trait C: B {} | ||
| impl<T> C for T {} | ||
|
|
||
| fn main() { |
There was a problem hiding this comment.
We have rustc_dump_predicates, which we could use to verify the predicates are getting lowered as we expect. It would be good to use that in this (or another) test.
There was a problem hiding this comment.
So, this test probably doesn't make sense here, right? This is not about method resolution.
| /// supertrait of another candidate trait. | ||
| /// | ||
| /// This implements RFC #3624. | ||
| fn collapse_candidates_to_subtrait_pick( |
There was a problem hiding this comment.
I think docs here and for collapse_candidates_to_subtrait_pick in method::probe should link to each other to help ensure they are kept up to date.
It's kind of unfortunate we can't have some shared code here.
| continue; | ||
| } | ||
|
|
||
| // This pick is not a supertrait of the `child_pick`. |
There was a problem hiding this comment.
I have a commit with some cleanup that I'm about to push - but child_pick doesn't exist. Should be child_trait. Here and below.
rustbot
added
S-waiting-on-author
5 participants
This makes type-level name resolution (used primarily for associated types) also prefer subtrait items to supertrait items in case of ambiguity.
This addresses the concern from #148605 about the inconsistency with name resolution in expressions and method calls.