Uh oh!
There was an error while loading. Please reload this page.
Rust: Disambiguate types inferred from trait bounds - #21464
Conversation
Uh oh!
There was an error while loading. Please reload this page.
4d48515 to
902e953CompareUh oh!
There was an error while loading. Please reload this page.
902e953 to
1dbd888CompareUh oh!
There was an error while loading. Please reload this page.
1dbd888 to
7a5a89aCompareUh oh!
There was an error while loading. Please reload this page.
9300d8d to
e14936fCompare98a7ca6 to
be2a49bCompareUh oh!
There was an error while loading. Please reload this page.
be2a49b to
03a444dCompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
b33051e to
c5af080CompareThere was a problem hiding this comment.
Pull request overview
This PR improves Rust type inference in cases where multiple trait impls overlap on the same receiver type but differ in trait type arguments, by lifting disambiguation logic into the shared type inference library so it applies both to <Foo as Bar<...>>::Assoc path resolution and to trait-bound constraints on function type parameters.
Changes:
- Generalize constraint satisfaction in the shared type inference library to support non-
Typeconstraints and optional type-parameter matching for disambiguation. - Extend Rust’s internal type inference to use the generalized shared machinery (including sibling-impl ambiguity checks) and refine associated-type string rendering for inherited associated types.
- Add/adjust Rust library tests to cover overlapping impls from trait bounds and update expected results.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/util/codeql/util/UnboundList.qll | Adds a more efficient prefix check helper used by new disambiguation logic. |
| shared/typeinference/codeql/typeinference/internal/TypeInference.qll | Generalizes SatisfiesConstraint and introduces type-matching-enabled disambiguation support. |
| rust/ql/lib/codeql/rust/internal/typeinference/TypeMention.qll | Adapts <Type as Trait>::Assoc resolution to use the shared disambiguation approach. |
| rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll | Refactors Rust instantiation of shared type inference modules and updates call sites to new APIs. |
| rust/ql/lib/codeql/rust/internal/typeinference/FunctionOverloading.qll | Refactors sibling-impl detection to be parameterized over type-mention resolution kind. |
| rust/ql/lib/codeql/rust/internal/typeinference/BlanketImplementation.qll | Switches blanket-constraint satisfaction to the new SatisfiesType wrapper. |
| rust/ql/lib/codeql/rust/internal/typeinference/Type.qll | Adjusts associated-type parameter representation and stringification (incl. inherited-from info). |
| rust/ql/test/library-tests/type-inference/overloading.rs | Adds a regression test module covering overlapping impls via trait bounds. |
| rust/ql/test/library-tests/type-inference/type-inference.expected | Updates expected inference output to reflect the improved disambiguation. |
Comments suppressed due to low confidence (1)
rust/ql/lib/codeql/rust/internal/typeinference/FunctionOverloading.qll:66
- Typo in comment: "superflous" should be "superfluous".
// In principle the second conjunct below should be superflous, but we still
// have ill-formed type mentions for types that we don't understand. For
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
b08dbe0 to
c607500Comparec607500 to
7fc1d53CompareUh oh!
There was an error while loading. Please reload this page.
Preliminary work needed for #21206.
In certain cases where a type implements the same trait but with different type arguments, we may infer overlapping types from trait bounds (see examples in first commit).
In order to disambiguate, this PR takes the same approach as #21420, but lifting the idea from that PR (implemented inside the
getPathConcreteAssocTypeAtprediate inTypeMention.qll) into the shared type inference library, so it applies to both resolution of<Foo as Bar<...>>paths as well as constraints on type parameters inside functions. In order to do this, the existingSatisfiesConstraintmodule is generalized from constraints beingTypes to beingHasTypeTreeSigs, for example inwe will now restrict applicable traits to only those that are compatible with a second type argument being
i32.The old
SatisfiesConstraintmodule is replaced with a newSatisfiesTypemodule, which is a simple wrapper aroundSatisfiesConstraint.Moreover, since we may actually know the instantiation of
T1at a given call tofooabove, we can restrict further based on that type, which means a further generalization ofSatisfiesConstraintto a newSatisfiesConstraintWithTypeMatchingmodule.SatisfiesConstraintis then itself also a simple wrapper (aroundSatisfiesConstraintWithTypeMatching).