-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Rust: Handle more explicit type arguments in type inference #19847
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
Rust: Handle more explicit type arguments in type inference #19847
Conversation
9403c2d
to
99bc945
Compare
99bc945
to
e0f16cf
Compare
d274c4a
to
c141f41
Compare
c141f41
to
f7195f0
Compare
@@ -1,2 +1,3 @@ | |||
illFormedTypeMention | |||
| macro_expansion.rs:99:7:99:19 | MyDeriveUnion | | |||
| macro_expansion.rs:99:7:99:19 | MyDeriveUnion | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are only a handful in the tests, but there are a lot of new type inference inconsistencies in the DCA run. Can we do / plan to do anything about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch; I have pushed a commit that should remove these overlapping results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing this.
I'm happy with the DCA results and tests - but I didn't get far figuring out how the code changes actually work. Up to you if you want to try and explain the changes broken down in more detail, or wait for someone else (e.g. @paldepind).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to have it merged soonish, as I have other PRs lined up that build on top.
04d8b3f
to
2924faf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me.
This PR improves handling of explicit type argument in type inference. For example, in
we are now able to correctly infer the type of
Foo::<i32>::default()
.The implementation works by treating type qualifiers as
self
arguments, and then the type of the qualifier is matched against theSelf
type of the call target, just like we do for normal arguments. This also means that for associated functions, likedefault
above, we pretend that there is aself
parameter for the sake of matching up types.Additionally, this PR also fixes an issue where type aliases were not resolved correctly, via a reimplementation of
TypeMention.resolveTypeAt
.DCA looks good; we get extra call targets at an acceptable slowdown.