Java/Cfg: Introduce new shared CFG library and replace the Java CFG.#21290
Java/Cfg: Introduce new shared CFG library and replace the Java CFG.#21290aschackmull wants to merge 32 commits intogithub:mainfrom
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
aschackmull
force-pushed
the
cfg/new-shared
branch
6 times, most recently
from
f54bdf4 to
0e7c3ba
Compare
aschackmull
force-pushed
the
cfg/new-shared
branch
4 times, most recently
from
3437f86 to
b90ee55
Compare
aschackmull
force-pushed
the
cfg/new-shared
branch
from
b90ee55 to
47a45ce
Compare
aschackmull
force-pushed
the
cfg/new-shared
branch
6 times, most recently
from
df51520 to
75b9346
Compare
… exception precision.
…are already in place, not CFG tests.
…YieldCompletions.
aschackmull
force-pushed
the
cfg/new-shared
branch
from
75b9346 to
94121f1
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new shared CFG library that encapsulates control flow semantics of common AST constructs and replaces the Java CFG implementation with an instantiation of this library. The change enables modular, non-recursive control flow definitions that integrate Java-specific semantics seamlessly with shared components.
Changes:
- Introduces a new
ConditionKindabstraction to classify different conditional successor types (Boolean, Nullness, Matching, Emptiness) - Adds performance optimizations to Guards.qll using
pragma[inline_late]andbindingsetannotations - Replaces the Java CFG implementation with a shared library instantiation
- Updates numerous test expectations to reflect the new CFG structure with additional synthetic nodes
- Removes deprecated
Completion.qlland updates SSA, dataflow, and guards implementations - Adds test utility modules for working with AST-based CFG views
Reviewed changes
Copilot reviewed 140 out of 143 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| shared/controlflow/codeql/controlflow/SuccessorType.qll | Adds ConditionKind abstraction and getDual() method for conditional successors |
| shared/controlflow/codeql/controlflow/Guards.qll | Adds strictlyDominatesCheck helper with pragma annotations for performance |
| shared/controlflow/codeql/controlflow/BasicBlock.qll | Adds getEnclosingCallable() method as alias for getScope() |
| java/ql/test/**/*.expected | Updates test expectations to reflect new CFG nodes and edges |
| java/ql/test/**/*.ql | Updates test queries to use AST-based CFG successors |
| java/ql/test/**/FalseSuccessors.* | Removes FalseSuccessors test files (no longer needed) |
| java/ql/lib/semmle/code/java/metrics/MetricCallable.qll | Updates switch case analysis to use AST structure instead of CFG |
| java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll | Removes hasDominanceInformation checks and updates CFG node access |
| java/ql/lib/semmle/code/java/dataflow/TypeFlow.qll | Fixes instanceof disjunction analysis for single-predecessor cases |
| java/ql/lib/semmle/code/java/controlflow/Guards.qll | Simplifies switch case edge detection using MatchingSuccessor |
| java/ql/lib/utils/test/*.qll | Adds test utilities for AST-based CFG views |
| java/ql/lib/change-notes/2026-02-18-cfg.md | Documents breaking changes |
owen-mc
left a comment
owen-mc
left a comment
There was a problem hiding this comment.
I haven't looked at it all in detail.
| * An example of this are resource declarations in Java's try-with-resources | ||
| * statement. | ||
| */ | ||
| default AstNode getTryInit(TryStmt try, int index) { none() } |
There was a problem hiding this comment.
Out of interest, why not make this a member predicate on TryStmt, which is clearly marked as being optional?
There was a problem hiding this comment.
Because most languages don't need it and making it a member means that it can't have a default implementation, so it ends up mandatory and ruins any chances that a language-specific instantiation can provide TryStmt as a simple alias.
There was a problem hiding this comment.
Ah. I didn't know it couldn't have a default implementation as a member predicate.
| * Gets an integer indicating the control flow order of a case within a switch. | ||
| * This is most often the same as the AST order, but can be different in some | ||
| * languages if the language allows a default case to appear before other | ||
| * cases. |
There was a problem hiding this comment.
I see that this doesn't have to be contiguous. It would be nice to state that here somehow.
| * Gets an integer indicating the control flow order of a case within a switch. | |
| * This is most often the same as the AST order, but can be different in some | |
| * languages if the language allows a default case to appear before other | |
| * cases. | |
| * Gets an integer indicating the control flow order of a case within a switch. | |
| * This is most often the same as the AST order, but can be different in some | |
| * languages if the language allows a default case to appear before other | |
| * cases. | |
| * | |
| * The values do not need to be contiguous; only the relative ordering matters. |
| * Holds if `n` is in a conditional context of kind `kind`. For example, | ||
| * the left-hand side of a short-circuiting `&&` expression is in a | ||
| * boolean conditional context. |
There was a problem hiding this comment.
I suggest making it clearer that this predicate is for adding extra cases on top of the ones already included by default. I also noticed this for beginAbruptCompletion, and there are probably other places.
| or | ||
| result.getIndex() <= getFirstPatternCase(switch).getIndex() | ||
| /** Holds if this catch clause catches all exceptions. */ | ||
| predicate catchAll(Ast::CatchClause catch) { |
There was a problem hiding this comment.
It feels like catchAll and matchAll are out of order. I expected them below additionalNode.
2 participants
This PR introduces a new shared CFG library, which encapsulated the control flow semantics of the most common AST constructs. Additionally, the Java CFG is replaced with an instantiation of this new library.
Control flow semantics of individual AST nodes are given in a modular non-recursive way, which allows seamless integration of the few Java-specific control flow definitions with the shared parts by way of simple union.
Any existing Java libraries with implicit reliance on the exact structure of the CFG have been updated.
Commit-by-commit review is strongly encouraged. There are a few initial setup commits, then the big commit with the main change followed by a long tail of commits to fix various tests and dependent Java libraries.