Skip to content

Shared CFG: check abrupt completion origins - #22423

Draft
yoff wants to merge 2 commits into
github:mainfrom
yoff:yoff-abrupt-origin-consistency
Draft

Shared CFG: check abrupt completion origins#22423
yoff wants to merge 2 commits into
github:mainfrom
yoff:yoff-abrupt-origin-consistency

Conversation

@yoff

@yoff yoff commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add invalidAbruptCompletionOrigin to the shared CFG consistency checks;
  • validate only language-supplied Input2::beginAbruptCompletion origins, requiring each origin to be either node.isIn(ast) or node.isAdditional(ast, _);
  • register the check in consistencyOverview, which is compiled and run by the existing consistency-query harnesses for every current consumer: Java, C#, and Python.

This intentionally does not inspect the shared library's combined private beginAbruptCompletion, whose unmatched final catch-clause completion legitimately originates at isAfterValue.

Motivation

This follows the consistency-check suggestion from review of #22380: #22380 (comment). The change is independent of #22380 and contains no Python-specific CFG behavior change.

Validation

  • codeql test run --search-path=. --consistency-queries java/ql/consistency-queries -- java/ql/test/library-tests/controlflow/basic — 27 tests passed
  • codeql test run --search-path=. --consistency-queries csharp/ql/consistency-queries -- csharp/ql/test/library-tests/controlflow/graph — 12 tests passed
  • codeql test run --search-path=. --consistency-queries python/ql/consistency-queries -- python/ql/test/library-tests/ControlFlow/successors — 6 tests passed

No invalid abrupt-completion origins or unrelated expected-output churn were found in the three consuming languages.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 25, 2026 12:30
@yoff
yoff requested review from a team as code owners August 25, 2026 12:30

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 validation ensuring language-supplied abrupt completions originate from their associated in/additional node.

Changes:

  • Registers invalidAbruptCompletionOrigin in CFG consistency checks.
  • Adds generated regression coverage for Java, C#, and Python consumers.
Show a summary per file
File Description
shared/controlflow/codeql/controlflow/ControlFlowGraph.qll Implements and registers the consistency check.
python/ql/test/library-tests/ControlFlow/successors/InvalidAbruptCompletionOrigin.ql Adds Python validation.
python/ql/test/library-tests/ControlFlow/successors/InvalidAbruptCompletionOrigin.expected Records expected Python results.
java/ql/test/library-tests/controlflow/basic/InvalidAbruptCompletionOrigin.ql Adds Java validation.
java/ql/test/library-tests/controlflow/basic/InvalidAbruptCompletionOrigin.expected Records expected Java results.
csharp/ql/test/library-tests/controlflow/graph/InvalidAbruptCompletionOrigin.ql Adds C# validation.
csharp/ql/test/library-tests/controlflow/graph/InvalidAbruptCompletionOrigin.expected Records expected C# results.

Review details

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

  • Files reviewed: 7/7 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@aschackmull

Copy link
Copy Markdown
Contributor

All of the added qltests and corresponding expected files are unnecessary, since this is a consistency query.

@yoff
yoff marked this pull request as draft August 25, 2026 13:14
@yoff

yoff commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, this was meant to be a draft. It is currently completely copilot, so I need to look it over.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: cdf52e0a-b37e-4fd9-a902-420148ff5ef8
@yoff yoff added the no-change-note-required This PR does not need a change note label Sep 1, 2026
@yoff

yoff commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Addressed: removed the standalone qltests and generated expected files. The existing Java, C#, and Python consistency-query harnesses compile and run the new check; their targeted suites pass 27/27, 12/12, and 6/6 respectively.

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.

3 participants