Rust: Make impl blocks only give rise to direct trait implementation #20847
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the Rust type inference logic to ensure that impl blocks only establish direct trait implementations rather than transitive ones. The change introduces a transitive boolean parameter to track whether constraint satisfaction should propagate transitively.
Key Changes
- Added a
transitiveparameter toconditionSatisfiesConstraintpredicate to distinguish direct vs. transitive trait implementations - Set
transitive=falsefor impl blocks implementing traits, andtransitive=truefor other constraint types (supertraits, trait bounds, etc.) - Updated function type resolution logic to properly handle trait constraints with the new transitivity semantics
Reviewed Changes
Copilot reviewed 8 out of 8 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 transitive parameter to core constraint satisfaction predicate; fixed typo in documentation |
| rust/ql/lib/codeql/rust/internal/TypeInference.qll | Implemented transitive parameter logic for Rust, setting it to false for impl blocks and true for other constraints |
| rust/ql/lib/codeql/rust/internal/typeinference/FunctionType.qll | Updated associated function type resolution to handle transitive constraints; refined type parameter specialization logic |
| rust/ql/test/library-tests/type-inference/main.rs | Removed SPURIOUS annotations from test comments where spurious call targets were eliminated |
| rust/ql/test/library-tests/type-inference/type-inference.expected | Removed spurious type inference results for reference types |
| rust/ql/test/library-tests/type-inference/CONSISTENCY/PathResolutionConsistency.expected | Removed multiple call target warnings that are now resolved |
| rust/ql/test/library-tests/dataflow/sources/stdin/CONSISTENCY/PathResolutionConsistency.expected | Removed resolved multiple call target warning |
| rust/ql/test/library-tests/dataflow/models/CONSISTENCY/PathResolutionConsistency.expected | Removed resolved multiple call target warning |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bindingset[abs, constraint, tp] | ||
| private Type getTraitConstraintTypeAt( | ||
| TypeMention condition, TypeMention constraint, TypeParameter tp, TypePath path | ||
| TypeAbstraction abs, TypeMention constraint, TypeParameter tp, TypePath path | ||
| ) { | ||
| BaseTypes::conditionSatisfiesConstraintTypeAt(_, condition, constraint, | ||
| BaseTypes::conditionSatisfiesConstraintTypeAt(abs, _, constraint, |
There was a problem hiding this comment.
The bindingset annotation specifies abs, constraint, and tp, but the function call on line 92 passes _ (wildcard) for the second parameter (which would be condition in the called predicate). The annotation should match the parameters that are actually bound in the function body. Since condition is now wildcarded and abs is used instead, verify that the bindingset annotation correctly reflects which parameters are expected to be bound by callers.
rust/ql/lib/codeql/rust/internal/typeinference/FunctionType.qll
Outdated
Show resolved
Hide resolved
|
@hvitved there was a bug with QLucie which prevernted from reporting back here, sorry about that. The problematic PR was reverted. I will manually trigger a new QLucie run now on this |
Co-authored-by: Copilot <[email protected]>
3 participants
Same as #20723, but with merge conflict resolved.