Skip to content

Shared CFG: allow destructuring in ForeachStmt#22047

Open
owen-mc wants to merge 1 commit into
github:mainfrom
owen-mc:shared/cfg/foreachstmt-multiple-variables
Open

Shared CFG: allow destructuring in ForeachStmt#22047
owen-mc wants to merge 1 commit into
github:mainfrom
owen-mc:shared/cfg/foreachstmt-multiple-variables

Conversation

@owen-mc

@owen-mc owen-mc commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

This commit is taken from #21614. The following commit in that PR shows how Go uses the changes in this commit to implement go's range statement using the shared library's ForeachStmt. Here are some examples showing the valid syntax for range statements:

for key, value := range mymap {
  fmt.Printf("mymap[%s] = %d\n", key, value)
}

for _, value = range array {
  fmt.Printf("array contains: %d\n", value)
}

for index, _ := range str {
  fmt.Printf("str[%d] = ?\n", index)
}

for value = range ch {
  fmt.Printf("value from channel: %d\n", value)
}

Design notes / alternatives considered

The shared library now lets a language opt in to a synthesized per-iteration ForeachElement node (foreachHasElementNode). The intended contract is:

The shared library owns the loop skeleton — testing the collection for emptiness, the [LoopHeader] join/branch point, the loop exit, and delivering control into [ForeachElement] at the start of each iteration. The language owns everything from the element node to the body, i.e. destructuring the current element into the loop variables.

This is a deliberate extension point, not an omission: when a language opts in, the shared library intentionally does not emit the getVariable() → body edge and instead hands control to the language at [ForeachElement].

Two more conventional designs were considered and rejected:

  1. Mirror C#: a single target node, with no change to the shared library.
    C# already supports tuple foreach (foreach (var (a, b) in xs)) without special CFG machinery — getVariable() returns a single VariableDeclTuple node and the generic expression CFG flows through its children to destructure. The equivalent for Go would be to present a single "target"/"pattern" node as getVariable(). This doesn't transfer: Go's range bindings are not a single grouping AST node. for k, v := range m is two independent LHS expressions plus an implicit extraction from the iterator, so there is no existing node to hand to the shared library. The ForeachElement node fills exactly this gap.

  2. Generalize getVariable() to an indexed getVariable(int i) and chain inside the shared library (variable 0 → variable 1 → … → body).
    This keeps all wiring in the shared library and produces a complete CFG with no language patching. It was prototyped and then rejected: Go's loop variables are not ordinary target expressions that can be evaluated in sequence — each is bound by extracting a component from the implicit current element. That per-variable extraction is language-specific and is more naturally owned by Go than modelled as a generic chain in the shared library.

Hence the chosen design: the shared library provides the loop skeleton plus a single opt-in element node, and Go wires ForeachElement → extract/assign → body itself. Java and C# do not opt in, keep their single getVariable(), and their generated CFG is byte-identical.

Copilot AI review requested due to automatic review settings June 24, 2026 19:00
@owen-mc owen-mc requested a review from a team as a code owner June 24, 2026 19:00
@owen-mc owen-mc added the no-change-note-required This PR does not need a change note label Jun 24, 2026

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 shared CFG library’s ForeachStmt handling to support languages whose foreach/range loops conceptually produce an implicit “current element” value that must be destructured into multiple loop variables (for example, Go’s for k, v := range m).

Changes:

  • Documents that ForeachStmt.getVariable() may be absent when a language needs multi-variable destructuring.
  • Adds an opt-in hook foreachHasElementNode(ForeachStmt) to request a synthesized per-iteration element node.
  • Updates shared CFG routing to enter the per-iteration element node (when opted in) and avoids routing getVariable() -> body in that mode.
Show a summary per file
File Description
shared/controlflow/codeql/controlflow/ControlFlowGraph.qll Adds an opt-in per-iteration foreach element node and updates CFG wiring/docs to support destructuring-based foreach semantics.

Copilot's findings

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

Comment thread shared/controlflow/codeql/controlflow/ControlFlowGraph.qll
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants