Rust: Unify logic in MethodResolution; remove TypeQualifierIsInstantiationOfImplSelf logic#21366
Merged
hvitved merged 2 commits intogithub:mainfrom Feb 26, 2026
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ntiationOfImplSelf` logic
hvitved
force-pushed
the
rust/type-inference-unify-method-resolution
branch
from
93d8651 to
de9b1ad
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Rust type inference/method resolution to treat type-qualified calls (for example Foo::m(x)) uniformly with receiver-style calls (x.m()), removing the bespoke TypeQualifierIsInstantiationOfImplSelf path and introducing an explicit “type qualifier” argument position.
Changes:
- Adds a new
ArgumentPositionvariant to represent type qualifiers and threads it through method-call candidate/receiver resolution. - Removes the special-case
TypeQualifierIsInstantiationOfImplSelflogic and generalizes existing “self”-based matching to “self or type qualifier”. - Updates type inference expected outputs to reflect additional inferred types/call edges.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/test/library-tests/type-inference/type-inference.expected | Updates expected results for type inference tests. |
| rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll | Refactors method resolution to unify receiver and type-qualifier handling; removes special-case qualifier logic. |
| rust/ql/lib/codeql/rust/internal/typeinference/FunctionType.qll | Introduces FunctionPosition.isTypeQualifier / isSelfOrTypeQualifier and uses qualifier position for assoc function typing. |
| rust/ql/lib/codeql/rust/internal/typeinference/FunctionOverloading.qll | Adjusts overload-resolution dependence logic to exclude both self and type qualifier positions. |
| rust/ql/lib/codeql/rust/elements/internal/InvocationExprImpl.qll | Adds TTypeQualifierArgumentPosition and exposes ArgumentPosition.isTypeQualifier(). |
Comment on lines
89
to
91
| // We always include the type qualifer position, even for non-methods, where it is used | ||
| // to match type qualifiers against the `impl` or trait type, such as in `Vec::new`. | ||
| (exists(pos.getTypeMention(f)) or pos.isSelf()) | ||
| (exists(pos.getTypeMention(f)) or pos.isTypeQualifier()) |
There was a problem hiding this comment.
Typo in comment: "type qualifer" should be "type qualifier".
| /** | ||
| * Holds if this call position is a type qualifier, that is, not an actual | ||
| * argument, but rather an annotation that is needed to resolve the call target, | ||
| * just like actual arguments may be need to resolve the call target. |
There was a problem hiding this comment.
Grammar in comment: "may be need" should be "may be needed".
Suggested change
| * just like actual arguments may be need to resolve the call target. | |
| * just like actual arguments may be needed to resolve the call target. |
paldepind
requested changes
Feb 26, 2026
| /** | ||
| * Holds if this call position is a type qualifier, that is, not an actual | ||
| * argument, but rather an annotation that is needed to resolve the call target, | ||
| * just like actual arguments may be need to resolve the call target. |
Contributor
There was a problem hiding this comment.
Maybe adding an example would be useful here?
Vec<i64>::new()
// ^^^^^^^^ type qualifier
paldepind
approved these changes
Feb 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR gets rid of the special
TypeQualifierIsInstantiationOfImplSelflogic for resolving method calls using type qualifiers such asFoo::m(x). The solution is to introduce another kind ofArgumentPositionfor representing type qualifiers, and then generalize existing logic that was hard-coded toselfpositions to also cover type qualifier positions, which allows us to handleFoo::m(x)andx.m()uniformly.DCA is great; we gain quite a lot of new call edges without significant performance cost.