Rust: Compute incompatible blanket implementations#20612
Rust: Compute incompatible blanket implementations#20612hvitved merged 3 commits intogithub:mainfrom
Conversation
da7efb8 to
2c6e29a
Compare
9976877 to
9e0f977
Compare
9e0f977 to
0e885e9
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements support for identifying invalid blanket implementation targets in Rust type inference by introducing a monotonic approximation to determine when a type tree does not satisfy implementation constraints.
Key changes:
- Added
isNotInstantiationOfandsatisfiesNotConstraintpredicates to provide monotonic approximations for ruling out incompatible implementations - Extended method and function resolution to check blanket-like implementations against constraint satisfaction
- Added comprehensive test cases for blanket-like implementations including edge cases with multiple trait bounds
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 |
|---|---|
| shared/typeinference/codeql/typeinference/internal/TypeInference.qll | Added core logic for monotonic constraint negation checking with isNotInstantiationOf, hasNotConstraintMention, and satisfiesNotConstraint predicates |
| rust/ql/lib/codeql/rust/internal/TypeInference.qll | Updated method and function resolution to handle blanket-like implementations by renaming predicates and adding incompatibility checks |
| rust/ql/lib/codeql/rust/internal/typeinference/BlanketImplementation.qll | Added satisfiesNotBlanketConstraint predicate to identify when argument types fail blanket constraints |
| rust/ql/test/library-tests/type-inference/blanket_impl.rs | Added test module with scenarios covering trait implementations, blanket implementations, and edge cases |
| rust/ql/test/library-tests/type-inference/type-inference.expected | Updated expected test results with new type inference entries for blanket-like implementations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| functionInfoBlanketLike(m, name, arity, impl, trait, pos, selfType, blanketPath, | ||
| blanketTypeParam) and |
There was a problem hiding this comment.
[nitpick] The line break in the middle of the function call arguments reduces readability. Consider keeping all parameters on the same line or formatting consistently with other similar calls in the file.
| functionInfoBlanketLike(m, name, arity, impl, trait, pos, selfType, blanketPath, | |
| blanketTypeParam) and | |
| functionInfoBlanketLike(m, name, arity, impl, trait, pos, selfType, blanketPath, blanketTypeParam) and |
paldepind
left a comment
There was a problem hiding this comment.
Really great improvements in DCA! QL looks good, my comments are only around names and documentation :)
shared/typeinference/codeql/typeinference/internal/TypeInference.qll
Outdated
Show resolved
Hide resolved
rust/ql/lib/codeql/rust/internal/typeinference/BlanketImplementation.qll
Outdated
Show resolved
Hide resolved
paldepind
left a comment
There was a problem hiding this comment.
Thanks for addressing my comments. I think it would be worthwhile addressing the Copilot one as well: #20612 (comment).
This PR follows up on #20282, by implementing support for identifying invalid blanket implementation targets.
A blanket implementation target can be dismissed when the constraint on the type parameter is not satisfied, and this has to be defined in a monotonic way, just like we had to for non-blanket implementations in #20282.
DCA shows that we recover performance on
gluon, and partially onpeace.