Open
Conversation
87447fb to
d6ff5b1
Compare
Consider two implementations of the same trait to be siblings when the type being implemented by one is an instantiation of the type being implemented by the other.
d6ff5b1 to
e60275c
Compare
implHasAmbiguousSiblingAtimplSiblings
Contributor
There was a problem hiding this comment.
Pull request overview
Refines Rust trait-impl sibling detection in the type inference / overload resolution logic so that impls are treated as siblings when one Self type is an instantiation of the other, reducing inconsistent resolution outcomes.
Changes:
- Extend shared type inference infrastructure with an
isPseudoTypehook to ignore pseudo types during non-instantiation checks. - Update Rust’s
implSiblingslogic to use instantiation checks rather than syntactic equality ofSelftype mentions. - Adjust Rust type-inference regression tests and expected output to match the refined sibling behavior.
Show a summary per file
| File | Description |
|---|---|
| shared/typeinference/codeql/typeinference/internal/TypeInference.qll | Adds isPseudoType to the shared input signature and uses it to ignore pseudo types in isNotInstantiationOf. |
| rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll | Implements isPseudoType for Rust (treating UnknownType/NeverType as pseudo). |
| rust/ql/lib/codeql/rust/internal/typeinference/FunctionOverloading.qll | Refines implSiblings to consider instantiation relationships between implemented Self types; updates related logic/docs. |
| rust/ql/test/library-tests/type-inference/regressions.rs | Updates regression annotation to reflect the corrected inferred type after sibling refinement. |
| rust/ql/test/library-tests/type-inference/type-inference.expected | Updates expected test output to match new inference/resolution results. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 1
Comment on lines
184
to
188
| * // ^ `path` = "T" | ||
| * } | ||
| * impl MyAdd<i64> for i64 { | ||
| * fn method(&self, value: Foo<i64>) -> Self { ... } | ||
| * // ^^^ `type` = i64 |
There was a problem hiding this comment.
In this documentation example, the trait is named MyTrait, but the subsequent impl line uses MyAdd<i64> for i64. This looks like a copy/paste typo and makes the example harder to follow; consider using the same trait name consistently throughout the snippet.
See below for a potential fix:
* impl MyTrait<i64> for i64 {
geoffw0
reviewed
Apr 23, 2026
Contributor
geoffw0
left a comment
There was a problem hiding this comment.
Results LGTM. Where should I start to understand the implementation?
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.
#21206 follow-up.
Consider two implementations of the same trait to be siblings when the type being implemented by one is an instantiation of the type being implemented by the other.
For example, in
the pairs
(I1, I2),(I3, I5), and(I4, I5)are siblings, but not(I3, I4).Before this PR, only
(I1, I2)were considered siblings.DCA is fine; while we do loose some call targets, we also fix inconsistencies, most notably we reduce
Nodes With Type At Length Limit.