Unified: Add control flow graph. - #22479
Open
aschackmull wants to merge 1 commit into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| AstNode getUpdate(int index) { none() } | ||
| } | ||
|
|
||
| class ForeachStmt extends LoopStmt instanceof U::ForEachStmt { |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Foreach, guard-statement, and closure-capture paths currently produce incorrect or unreachable control flow.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
 |
unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll — There are no edge-expectation tests for this new CFG implementation. The consistency query catches… |
 |
unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll — Excluding every child of a callable drops closure capture initializers from the CFG. FunctionExpr… |
What changed in this PR
Adds an initial unified-language CFG implementation with IDE visualization and consistency checks.
Changes:
- Adapts unified AST nodes to the shared control-flow framework.
- Exposes CFG APIs and adds the required dependency.
- Adds CFG visualization and consistency queries.
| File | Description |
|---|---|
unified/ql/lib/unified.qll |
Exposes CFG APIs. |
unified/ql/lib/qlpack.yml |
Adds the control-flow dependency. |
unified/ql/lib/ide-contextual-queries/printCfg.ql |
Adds IDE CFG visualization. |
unified/ql/lib/codeql/unified/internal/FacadeAst.qll |
Adds block-last-statement support. |
unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll |
Implements unified CFG modeling. |
unified/ql/consistency-queries/qlpack.yml |
Defines the consistency-query pack. |
unified/ql/consistency-queries/CfgConsistency.ql |
Enables shared CFG consistency checks. |
Suppressed comments (3)
unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll:85
GuardIfStmtis currently left to the default child ordering, which evaluates its condition and then itselseblock unconditionally. The Swift extractor emits this node for everyguard … elsestatement, so the CFG incorrectly enters theelseblock even on the true branch. Model it as the sharedIfStmtshape, with nothenbranch, so true flow continues after the guard and false flow enterselse.
class IfStmt extends Stmt {
IfStmt() { none() }
unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll:132
- This mapping is empty:
getPattern()returnsPattern, while this method requiresExpr(the schema makesexprandpatternseparate alternatives underexpr_or_pattern). Consequently, the shared foreach edge from a non-empty collection togetVariable()has no target, so every non-empty foreach path dead-ends before its body. Generalize the shared foreach-variable API toAstNodeand return the pattern through it.
Expr getVariable() { result = super.getPattern() }
unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll:129
- The foreach guard is omitted from the explicit loop CFG. Although it remains an AST child, the shared implementation disables default child traversal once
ForeachStmthas explicit steps, so the guard is unreachable and the body is entered without testing it. Extend the shared foreach signature/steps to evaluate the guard each iteration and branch back to the loop header when false.
// TODO support foreach guard
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+11
to
+15
| private module Cfg0 = Make0<Location, Ast>; | ||
|
|
||
| private module Cfg1 = Make1<Input>; | ||
|
|
||
| private module Cfg2 = Make2<Input>; |
Comment on lines
+31
to
+35
| result.getParent() = n and | ||
| result.getParentIndex() = index and | ||
| not n instanceof Callable and | ||
| not skipControlFlow(n) and | ||
| not skipControlFlow(result) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a very rough initial version, but it provides the basic functionality including "View CFG" support and consistency queries.