-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Rust: Also apply adjustedAccessType
in RelevantAccess
#19729
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
Rust: Also apply adjustedAccessType
in RelevantAccess
#19729
Conversation
e215bef
to
4739c88
Compare
4739c88
to
18392a0
Compare
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 extends the RelevantAccess
machinery to account for an explicit target
declaration when computing adjusted access types, replacing direct calls to getInferredType
. It also updates the Rust type‐inference tests and main.rs comments to reflect now‐included unwrap
calls and associated‐type bindings.
- Broadened
relevantAccessConstraint
andMkRelevantAccess
to carry aDeclaration target
so thatadjustedAccessType
can be applied - Switched from
a.getInferredType
toadjustedAccessType
ingetTypeAt
andhasTypeConstraint
- Updated test expectations and main.rs to register the previously missing
unwrap
method and associated‐type occurrences
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
shared/typeinference/codeql/typeinference/internal/TypeInference.qll | Added target parameter and replaced getInferredType with adjustedAccessType |
rust/ql/test/library-tests/type-inference/type-inference.expected | Added expected entries for the unwrap call and associated parameter inference |
rust/ql/test/library-tests/type-inference/main.rs | Updated comment to expect method=unwrap |
Comments suppressed due to low confidence (1)
shared/typeinference/codeql/typeinference/internal/TypeInference.qll:1024
- The
toString
method forRelevantAccess
doesn’t include the newtarget
field, which could be valuable for debugging; consider appendingtarget.toString()
to the output.
string toString() {
@@ -1076,7 +1078,7 @@ module Make1<LocationSig Location, InputSig1<Location> Input1> { | |||
TypeAbstraction abs, TypeMention sub, TypePath path, Type t | |||
) { | |||
exists(TypeMention constraintMention | | |||
at = MkRelevantAccess(a, apos, prefix) and | |||
at = MkRelevantAccess(a, _, apos, prefix) 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.
[nitpick] Using an underscore for the target
parameter in MkRelevantAccess
can obscure which argument is being ignored; consider naming this dummy parameter explicitly (e.g., dummyTarget
) to improve readability.
at = MkRelevantAccess(a, _, apos, prefix) and | |
at = MkRelevantAccess(a, dummyTarget, apos, prefix) and |
Copilot uses AI. Check for mistakes.
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.
even though I can't fully understand how this fix works, I can see from the fixed MISSING
in that test that it does work 👍
No description provided.