Skip to content

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

Merged

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Jun 11, 2025

No description provided.

@hvitved hvitved force-pushed the rust/type-inference-adjust-type-relevant-access branch from e215bef to 4739c88 Compare June 11, 2025 12:51
@github-actions github-actions bot added the Rust Pull requests that update Rust code label Jun 11, 2025
@hvitved hvitved force-pushed the rust/type-inference-adjust-type-relevant-access branch from 4739c88 to 18392a0 Compare June 11, 2025 13:00
@hvitved hvitved marked this pull request as ready for review June 11, 2025 19:13
@Copilot Copilot AI review requested due to automatic review settings June 11, 2025 19:13
@hvitved hvitved requested a review from a team as a code owner June 11, 2025 19:13
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Jun 11, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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 and MkRelevantAccess to carry a Declaration target so that adjustedAccessType can be applied
  • Switched from a.getInferredType to adjustedAccessType in getTypeAt and hasTypeConstraint
  • 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 for RelevantAccess doesn’t include the new target field, which could be valuable for debugging; consider appending target.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
Copy link
Preview

Copilot AI Jun 11, 2025

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.

Suggested change
at = MkRelevantAccess(a, _, apos, prefix) and
at = MkRelevantAccess(a, dummyTarget, apos, prefix) and

Copilot uses AI. Check for mistakes.

@hvitved hvitved requested a review from paldepind June 12, 2025 07:02
Copy link
Contributor

@redsun82 redsun82 left a 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 👍

@hvitved hvitved merged commit 93fd6ec into github:main Jun 12, 2025
40 of 41 checks passed
@hvitved hvitved deleted the rust/type-inference-adjust-type-relevant-access branch June 12, 2025 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants