-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Rust: Fix type inference for explicit dereference with *
to the Deref
trait
#19820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the Rust type-inference engine to correctly handle explicit *
dereferences via the Deref
trait and to distinguish “certain” borrows, and it updates tests and internal QL logic accordingly.
- Add new dereference tests and update existing expected-results files to exercise explicit/implicit
Deref
calls. - Extend
CallExprBaseMatchingInput
to carry acertain
flag on borrows and adjustTypeInference.qll
to honor explicit*
in both argument and return positions. - Update internal QL modules (
OperationImpl.qll
,CallImpl.qll
) to propagate the new borrow flags and update test annotations for write‐formatted calls and dataflow.
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
rust/ql/test/query-tests/security/.../PathResolutionConsistency.expected | Add expected write_fmt paths for explicit deref consistency tests. |
rust/ql/test/library-tests/type-inference/main.rs | Introduce MyInt , ATrait , remove spurious annotations, register new dereference module. |
rust/ql/test/library-tests/type-inference/dereference.rs | New file with explicit/implicit deref tests through Deref trait. |
rust/ql/test/library-tests/type-inference/CONSISTENCY/... | Add consistency expectations for e1.deref() . |
rust/ql/test/library-tests/dataflow/.../DataFlowStep.expected | Insert unwrap receiver entries in local dataflow steps. |
rust/ql/test/library-tests/dataflow/global/viableCallable.expected | Add explicit * deref entries to viable callables. |
rust/ql/test/library-tests/dataflow/global/main.rs | Update sink comment to include hasTaintFlow . |
rust/ql/lib/codeql/rust/internal/TypeInference.qll | Track “certain” borrows in AccessPosition , adjust primitive handling of explicit * . |
rust/ql/lib/codeql/rust/elements/internal/OperationImpl.qll | Change deref operator’s borrows count from 0 to 1. |
rust/ql/lib/codeql/rust/elements/internal/CallImpl.qll | Expand implicitBorrowAt predicate to accept a certain flag. |
pathAdj = TypePath::cons(TRefTypeParameter(), path) and | ||
tAdj = t | ||
or | ||
apos.isBorrowed(false) and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using apos.isBorrowed(false)
still matches only borrowed arguments (with certain = false
) rather than the 'not borrowed' case. You should replace this clause with not apos.isBorrowed(_)
to correctly handle non-borrowed positions.
apos.isBorrowed(false) and | |
not apos.isBorrowed(_) and |
Copilot uses AI. Check for mistakes.
*
to the Deref
trait*
to the Deref
trait
c9943a2
to
6b2c125
Compare
This PR should fix type inference for explicit dereferencing.
&
types. Previously types in the receiver position where always dereferenced before method lookup. Now for a&T
we lookup methods both on&T
itself and onT
.*
in the desugaring of*
.*e
desugars to*Deref::deref(&e)
and two handle the*
after the call we need a special case in the return position of*
.Call
abstraction. For instance, sincea == b
desugars toEqual::eq(&a, &b)
so we can be certain thata
andb
are borrowed. Making these certain sidesteps some issues with the way we infer implicit borrows. For instance for an argument type of the form&T
we will attempt to infer a dereference and never infer an implicit borrow. But thederef
method on&T
is a actually a borrow from&T
to&&T
.