Skip to content

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented Nov 5, 2025

DCA is great: Improved performance on rust and a lot of rust/unused-variable false positives removed.

@hvitved hvitved added the no-change-note-required This PR does not need a change note label Nov 5, 2025
@github-actions github-actions bot added the Rust Pull requests that update Rust code label Nov 5, 2025
@hvitved hvitved marked this pull request as ready for review November 6, 2025 09:30
@hvitved hvitved requested a review from a team as a code owner November 6, 2025 09:30
Copilot AI review requested due to automatic review settings November 6, 2025 09:30
Copy link
Contributor

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 improves the handling of variable scoping in match arm guards with nested let patterns in Rust. The key change refactors how variable scopes are managed for match arms to properly handle cases where variables declared in the match arm pattern can be shadowed within the guard expression itself.

Key changes:

  • Refactored MatchArmScope from an abstract class with two implementations into two separate scope classes
  • Introduced ConditionScope for match arm guards to properly handle variables declared in nested let expressions within guards
  • Updated the variable declaration logic to correctly determine variable scope and ordering for match arm patterns

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

File Description
rust/ql/test/library-tests/variables/main.rs Added test case match_pattern16() demonstrating variable shadowing in match arm guards with nested let patterns
rust/ql/test/library-tests/variables/variables.expected Updated expected test results to include the new test case variables and their accesses
rust/ql/test/library-tests/variables/Ssa.expected Updated SSA analysis expected results for the new test case
rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll Refactored scope classes and variable declaration logic to properly handle match arm guard scoping

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@hvitved hvitved requested a review from geoffw0 November 6, 2025 12:11
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes LGTM.

The improvement to rust/unused-variable results is very welcome.

@hvitved hvitved merged commit 000f33f into github:main Nov 6, 2025
28 checks passed
@hvitved hvitved deleted the rust/variable-if-let-guard branch November 6, 2025 12:49
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