Uh oh!
There was an error while loading. Please reload this page.
Rust: Compute incompatible blanket implementations - #20612
Conversation
da7efb8 to
2c6e29aCompare9976877 to
9e0f977Compare9e0f977 to
0e885e9CompareThere 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.
Uh oh!
There was an error while loading. Please reload this page.
| 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 :)
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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).
Uh oh!
There was an error while loading. Please reload this page.
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.