Skip to content

Shared CFG: Make the init and update parts of a for loop statements#21880

Merged
owen-mc merged 3 commits into
github:mainfrom
owen-mc:shared/cfg/for-loop-stmt-init-update
May 21, 2026
Merged

Shared CFG: Make the init and update parts of a for loop statements#21880
owen-mc merged 3 commits into
github:mainfrom
owen-mc:shared/cfg/for-loop-stmt-init-update

Conversation

@owen-mc
Copy link
Copy Markdown
Contributor

@owen-mc owen-mc commented May 21, 2026

This is needed for Go.

@owen-mc owen-mc marked this pull request as ready for review May 21, 2026 13:49
@owen-mc owen-mc requested a review from a team as a code owner May 21, 2026 13:49
Copilot AI review requested due to automatic review settings May 21, 2026 13:49
@owen-mc owen-mc requested review from a team as code owners May 21, 2026 13:49
Copy link
Copy Markdown
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

Updates the shared Control Flow Graph AST signature so for-loop initializer and update components are represented as AST nodes (not necessarily expressions), enabling languages like Go where these parts can be statements.

Changes:

  • Generalize ForStmt.getInit/getUpdate in the shared CFG AST signature from Expr to AstNode.
  • Adjust Java CFG AST adapter to expose getInit/getUpdate with the new AstNode return type.
  • Adjust C# CFG AST adapter similarly (and align initializer access via super).
Show a summary per file
File Description
shared/controlflow/codeql/controlflow/ControlFlowGraph.qll Generalizes shared ForStmt init/update accessors to return AstNode and updates docstrings.
java/ql/lib/semmle/code/java/ControlFlowGraph.qll Wraps Java ForStmt to provide AstNode-typed getInit/getUpdate implementations matching the shared signature.
csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraph.qll Updates C# ForStmt adapter to return AstNode for init/update per the shared signature.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment on lines +121 to +128
/** Gets the initializer of the loop at the specified (zero-based) position, if any. */
AstNode getInit(int index);

/** Gets the boolean condition of this `for` loop. */
Expr getCondition();

/** Gets the update expression of this loop at the specified (zero-based) position, if any. */
Expr getUpdate(int index);
/** Gets the update of this loop at the specified (zero-based) position, if any. */
AstNode getUpdate(int index);
@owen-mc owen-mc added the no-change-note-required This PR does not need a change note label May 21, 2026
Copy link
Copy Markdown
Contributor

@aschackmull aschackmull left a comment

Choose a reason for hiding this comment

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

Right, there's no real reason that these need to be Exprs, so this change makes sense.

LGTM.

@owen-mc owen-mc merged commit 149bfd1 into github:main May 21, 2026
145 of 146 checks passed
@owen-mc owen-mc deleted the shared/cfg/for-loop-stmt-init-update branch May 21, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C# Java no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants