Skip to content

Shared CFG: add defaulted getWhileElse/getForeachElse to AstSig#21931

Merged
yoff merged 2 commits into
mainfrom
yoff/python-shared-cfg-loop-else
Jun 23, 2026
Merged

Shared CFG: add defaulted getWhileElse/getForeachElse to AstSig#21931
yoff merged 2 commits into
mainfrom
yoff/python-shared-cfg-loop-else

Conversation

@yoff

@yoff yoff commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Lifts the shared-controlflow changes out of #21921 so they can be reviewed independently. No behavioural change for existing languages (Java, C#, Ruby, Swift, Go, …) — both new predicates default to none() and the Make0 loop-edge case extensions only fire when a language overrides them.

Motivation

Python's upcoming new CFG library (#21921) needs to model while-else / for-else, where the else block runs when the loop condition becomes false (rather than via a break). The shared CFG didn't previously have predicates exposing those else blocks.

Changes

  • Adds defaulted getLoopElse(LoopStmt) to AstSig (shared/controlflow/codeql/controlflow/ControlFlowGraph.qll).
  • Extends Make0 to route through the else block if one exists.

Verification

Copilot AI review requested due to automatic review settings June 2, 2026 13:48
@yoff yoff requested a review from a team as a code owner June 2, 2026 13:48

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

Adds shared-CFG signature hooks for loop else blocks so languages that support while-else / for-else (notably Python) can expose that structure to the shared control-flow graph without impacting existing languages.

Changes:

  • Added defaulted AstSig.getWhileElse(WhileStmt) and AstSig.getForeachElse(ForeachStmt) predicates (default none()).
  • Updated shared CFG construction (Make0) to route certain loop-exit edges through an else block when provided by a language implementation.
  • Added a shared/controlflow change note documenting the new signature predicates.
Show a summary per file
File Description
shared/controlflow/codeql/controlflow/ControlFlowGraph.qll Extends the shared CFG AST signature with defaulted loop-else accessors and threads them into loop edge construction.
shared/controlflow/change-notes/2026-05-19-loop-else.md Documents the new defaulted AstSig predicates for loop else blocks.

Copilot's findings

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

---
category: feature
---
* The `AstSig` signature gains two new defaulted predicates `getWhileElse` and `getForeachElse`, allowing languages (like Python) to model `while-else` / `for-else` constructs where the `else` branch is taken when the loop condition becomes false (rather than via a `break`). Existing languages that do not provide these predicates retain the previous behaviour.

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.

I wouldn't add a change note. This isn't a user-facing change.

@yoff yoff force-pushed the yoff/python-shared-cfg-loop-else branch from d558371 to 2eaf01c Compare June 5, 2026 07:24
@yoff yoff force-pushed the yoff/python-shared-cfg-loop-else branch from 2eaf01c to 4d2296d Compare June 5, 2026 08:12
default AstNode getTryElse(TryStmt try) { none() }

/**
* Gets the `else` block of this `while` loop statement, if any.

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.

Suggested change
* Gets the `else` block of this `while` loop statement, if any.
* Gets the `else` block of `while` loop statement `loop`, if any.

default AstNode getWhileElse(WhileStmt loop) { none() }

/**
* Gets the `else` block of this `foreach` loop statement, if any.

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.

Suggested change
* Gets the `else` block of this `foreach` loop statement, if any.
* Gets the `else` block of `foreach` loop statement `loop`, if any.

default AstNode getForeachElse(ForeachStmt loop) { none() }

/**
* Gets the type expression of this catch clause, if any.

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.

Suggested change
* Gets the type expression of this catch clause, if any.
* Gets the type expression of catch clause `catch`, if any.

exists(MatchingSuccessor t |
n1.isBefore(catchclause) and
(
n2.isBefore(getCatchType(catchclause))

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.

t is unbound in this disjunct.

*
* Only some languages (e.g. Python) support `while-else` constructs.
*/
default AstNode getWhileElse(WhileStmt loop) { none() }

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.

Let's stick to just one new predicate

Suggested change
default AstNode getWhileElse(WhileStmt loop) { none() }
default AstNode getLoopElse(LoopStmt loop) { none() }

The semantics of a loop-else block is mainly related to whether the loop exits via its condition or a break, so it's not really tied to the specifics of while vs. foreach.

* statically resolved, this defaults to `none()` and no CFG node
* is created.
*/
default Expr getCatchType(CatchClause catch) { none() }

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.

This is unrelated to the PR description - was this meant to go in a different PR?

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.

Also, unless absolutely necessary, I don't think this should be a point of parameterisation - rather, I think we ought to be able to come up with a CFG for catch that solves the needs of all languages. We should probably discuss this offline and put a fix in a different PR when we agree on the solution.

@yoff yoff force-pushed the yoff/python-shared-cfg-loop-else branch 2 times, most recently from 51e783f to 9e69738 Compare June 22, 2026 13:48
@yoff yoff requested review from aschackmull and hvitved June 22, 2026 14:14
Comment thread shared/controlflow/codeql/controlflow/ControlFlowGraph.qll Outdated
@yoff yoff added the no-change-note-required This PR does not need a change note label Jun 23, 2026
@yoff yoff requested a review from aschackmull June 23, 2026 10:40
nischal and others added 2 commits June 23, 2026 12:41
Adds a new defaulted signature predicates to the shared CFG library:

- getLoopElse: `else` block of a loop statement, if
  any (used by Python's `while-else` / `for-else` constructs).

The predicate defaults to `none()`, so behaviour is unchanged for any
language that doesn't override it (verified by re-running
java/ql/test/library-tests/controlflow/).

The Make0 succession rules are extended:
- WhileStmt/ForeachStmt: route the loop-exit edge through the else
  block before reaching the after-position.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Anders Schack-Mulligen <aschackmull@users.noreply.github.com>
@yoff yoff force-pushed the yoff/python-shared-cfg-loop-else branch from 215bd3d to 73ab3e6 Compare June 23, 2026 10:41
@yoff yoff merged commit 53cae68 into main Jun 23, 2026
132 checks passed
@yoff yoff deleted the yoff/python-shared-cfg-loop-else branch June 23, 2026 12:25
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.

6 participants