Skip to content

Rust: Also apply adjustedAccessType in RelevantAccess#19729

Merged
hvitved merged 1 commit into
github:mainfrom
hvitved:rust/type-inference-adjust-type-relevant-access
Jun 12, 2025
Merged

Rust: Also apply adjustedAccessType in RelevantAccess#19729
hvitved merged 1 commit into
github:mainfrom
hvitved:rust/type-inference-adjust-type-relevant-access

Conversation

@hvitved

@hvitved hvitved commented Jun 11, 2025

Copy link
Copy Markdown
Contributor

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 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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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() {

) {
exists(TypeMention constraintMention |
at = MkRelevantAccess(a, apos, prefix) and
at = MkRelevantAccess(a, _, apos, prefix) and

Copilot AI Jun 11, 2025

Copy link

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

@redsun82 redsun82 left a comment

Copy link
Copy Markdown
Contributor

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.

3 participants