Skip to content

Unified: Use the shared lib handling of LabeledStmt. - #22768

Open
aschackmull wants to merge 1 commit into
github:mainfrom
aschackmull:unified/cfg-labeledstmt
Open

aschackmull wants to merge 1 commit into
github:mainfrom
aschackmull:unified/cfg-labeledstmt

Conversation

@aschackmull

Copy link
Copy Markdown
Contributor

Minor simplification now that the shared CFG lib knows about labeled statementst.

@aschackmull
aschackmull requested a review from a team as a code owner October 7, 2026 09:44
@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Oct 7, 2026
Copilot AI balanced review requested due to automatic review settings October 7, 2026 09:44

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.

Copilot review overview

🟢 Approval recommended

The shared CFG implementation correctly handles enclosing labels through LabeledStmt.getStmt().

Review effort: Balanced
Findings: None

What changed in this PR

Simplifies Unified CFG label handling by delegating labeled statements to the shared control-flow library.

Changes:

  • Maps LabeledStmt directly to the Unified AST type.
  • Limits hasLabel to labels directly attached to AST nodes.
File Description
unified/​ql/​lib/​codeql/​unified/​internal/​ControlFlowGraph.qll Uses shared CFG handling for nested labeled statements.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@hvitved hvitved 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.

LGTM, do we already have CFG tests for labeled statements?

This branch has not been deployed

No deployments
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 Unified

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants