Skip to content

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

Merged
merged 4 commits into from
Jul 4, 2025

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Jun 23, 2025

This PR improves handling of explicit type argument in type inference. For example, in

struct Foo<T>(T);

impl<T : Default> Foo<T> {
  fn default() -> Self {
    Foo(Default::default())
  }
}

Foo::<i32>::default();

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 the Self type of the call target, just like we do for normal arguments. This also means that for associated functions, like default above, we pretend that there is a self 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.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Jun 23, 2025
@hvitved hvitved force-pushed the rust/type-inference-explicit-args branch 5 times, most recently from 9403c2d to 99bc945 Compare June 24, 2025 14:12
@hvitved hvitved force-pushed the rust/type-inference-explicit-args branch from 99bc945 to e0f16cf Compare June 30, 2025 18:12
@hvitved hvitved force-pushed the rust/type-inference-explicit-args branch 3 times, most recently from d274c4a to c141f41 Compare July 2, 2025 08:38
@hvitved hvitved force-pushed the rust/type-inference-explicit-args branch from c141f41 to f7195f0 Compare July 2, 2025 09:37
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Jul 3, 2025
@hvitved hvitved marked this pull request as ready for review July 3, 2025 08:31
@hvitved hvitved requested a review from a team as a code owner July 3, 2025 08:31
@@ -1,2 +1,3 @@
illFormedTypeMention
| macro_expansion.rs:99:7:99:19 | MyDeriveUnion |
| macro_expansion.rs:99:7:99:19 | MyDeriveUnion |
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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).

Copy link
Contributor Author

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.

@hvitved hvitved force-pushed the rust/type-inference-explicit-args branch from 04d8b3f to 2924faf Compare July 3, 2025 09:56
Copy link
Contributor

@aibaars aibaars left a 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.

@hvitved hvitved merged commit e33ddce into github:main Jul 4, 2025
19 checks passed
@hvitved hvitved deleted the rust/type-inference-explicit-args branch July 4, 2025 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants