Go: migrate control flow graph to shared CFG library #2 - #22182
Open
owen-mc wants to merge 37 commits into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates Go control-flow, SSA, and data-flow modeling to the shared CFG framework, including schema support and updated test baselines.
Changes:
- Integrates shared CFG/basic-block infrastructure and no-return modeling.
- Adds synthesized range-element nodes with upgrade/downgrade support.
- Updates framework models, inline annotations, and generated expectations.
Show a summary per file
| File | Description |
|---|---|
go/ql/test/query-tests/Security/CWE-918/RequestForgery.expected |
Updates data-flow node labels. |
go/ql/test/query-tests/Security/CWE-918/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/query-tests/Security/CWE-770/UncontrolledAllocationSize.expected |
Updates extraction-node labels. |
go/ql/test/query-tests/Security/CWE-770/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/query-tests/Security/CWE-643/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/query-tests/Security/CWE-640/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/query-tests/Security/CWE-601/OpenUrlRedirect/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/query-tests/Security/CWE-601/BadRedirectCheck/main.go |
Relocates source annotations. |
go/ql/test/query-tests/Security/CWE-601/BadRedirectCheck/cves.go |
Relocates source annotation. |
go/ql/test/query-tests/Security/CWE-601/BadRedirectCheck/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/query-tests/Security/CWE-601/BadRedirectCheck/BadRedirectCheck.go |
Relocates source annotation. |
go/ql/test/query-tests/Security/CWE-347/MissingJwtSignatureCheck.expected |
Updates SSA locations and labels. |
go/ql/test/query-tests/Security/CWE-347/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/query-tests/Security/CWE-327/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/query-tests/Security/CWE-326/InsufficientKeySize.expected |
Updates result ordering and SSA locations. |
go/ql/test/query-tests/Security/CWE-322/InsecureHostKeyCallback.expected |
Updates SSA and extraction nodes. |
go/ql/test/query-tests/Security/CWE-312/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/query-tests/Security/CWE-295/DisabledCertificateCheck/DisabledCertificateCheck.expected |
Updates assignment and literal labels. |
go/ql/test/query-tests/Security/CWE-190/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/query-tests/Security/CWE-117/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/query-tests/Security/CWE-089/StringBreak.expected |
Updates extraction-node labels. |
go/ql/test/query-tests/Security/CWE-089/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/query-tests/Security/CWE-089/CONSISTENCY/CfgConsistency.expected |
Adds CFG consistency baseline. |
go/ql/test/query-tests/Security/CWE-079/StoredXss.expected |
Updates extraction and SSA nodes. |
go/ql/test/query-tests/Security/CWE-079/stored.go |
Relocates source annotation. |
go/ql/test/query-tests/Security/CWE-079/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/query-tests/Security/CWE-078/StoredCommand.expected |
Updates extraction-node labels. |
go/ql/test/query-tests/Security/CWE-078/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/query-tests/Security/CWE-078/CommandInjection.expected |
Updates SSA source ranges. |
go/ql/test/query-tests/Security/CWE-022/UnsafeUnzipSymlink.expected |
Updates SSA locations. |
go/ql/test/query-tests/Security/CWE-022/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/query-tests/Security/CWE-020/MissingRegexpAnchor/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/query-tests/RedundantCode/UnreachableStatement/CONSISTENCY/CfgConsistency.expected |
Adds CFG consistency baseline. |
go/ql/test/query-tests/RedundantCode/RedundantRecover/tst.go |
Removes an obsolete alert annotation. |
go/ql/test/query-tests/RedundantCode/RedundantRecover/RedundantRecover.expected |
Updates recover-call results. |
go/ql/test/query-tests/RedundantCode/DeadStoreOfLocal/DeadStoreOfLocal.expected |
Updates assignment instruction labels. |
go/ql/test/query-tests/RedundantCode/DeadStoreOfLocal/CONSISTENCY/CfgConsistency.expected |
Adds CFG consistency baseline. |
go/ql/test/query-tests/RedundantCode/DeadStoreOfField/DeadStoreOfField.expected |
Updates assignment label. |
go/ql/test/query-tests/InconsistentCode/UnhandledCloseWritableHandle/CONSISTENCY/CfgConsistency.expected |
Adds defer CFG baseline. |
go/ql/test/query-tests/InconsistentCode/MissingErrorCheck/MissingErrorCheck.expected |
Updates SSA locations. |
go/ql/test/library-tests/semmle/go/Types/notype.ql |
Excludes synthesized valid types. |
go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.expected |
Updates dereference labels. |
go/ql/test/library-tests/semmle/go/Scopes/EntityWrite.expected |
Updates parameter-init nodes. |
go/ql/test/library-tests/semmle/go/PrintAst/PrintAstExcludeComments.expected |
Adds range-element AST nodes. |
go/ql/test/library-tests/semmle/go/PrintAst/PrintAst.expected |
Adds range-element AST nodes. |
go/ql/test/library-tests/semmle/go/PrintAst/CONSISTENCY/CfgConsistency.expected |
Adds CFG consistency baseline. |
go/ql/test/library-tests/semmle/go/IR/test.expected |
Updates extraction instruction labels. |
go/ql/test/library-tests/semmle/go/frameworks/Yaml/yaml.go |
Updates inline model expectations. |
go/ql/test/library-tests/semmle/go/frameworks/XNetHtml/SqlInjection.expected |
Updates extraction-node labels. |
go/ql/test/library-tests/semmle/go/frameworks/XNetHtml/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/library-tests/semmle/go/frameworks/WebSocket/RemoteFlowSources.expected |
Updates extraction-node labels. |
go/ql/test/library-tests/semmle/go/frameworks/WebSocket/Read.expected |
Updates extraction-node labels. |
go/ql/test/library-tests/semmle/go/frameworks/WebSocket/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference label. |
go/ql/test/library-tests/semmle/go/frameworks/Twirp/server/main.go |
Relocates handler/source annotations. |
go/ql/test/library-tests/semmle/go/frameworks/TaintSteps/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/library-tests/semmle/go/frameworks/SystemCommandExecutors/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference label. |
go/ql/test/library-tests/semmle/go/frameworks/Revel/test.expected |
Records inline expectation mismatch. |
go/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go |
Relocates response-body annotation. |
go/ql/test/library-tests/semmle/go/frameworks/Revel/OpenRedirect.expected |
Updates dereference labels. |
go/ql/test/library-tests/semmle/go/frameworks/Protobuf/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/library-tests/semmle/go/frameworks/Protobuf/CONSISTENCY/CfgConsistency.expected |
Adds CFG consistency baseline. |
go/ql/test/library-tests/semmle/go/frameworks/gqlgen/graph/schema.resolvers.go |
Relocates resolver annotation. |
go/ql/test/library-tests/semmle/go/frameworks/GoMicro/main.go |
Relocates request annotation. |
go/ql/test/library-tests/semmle/go/frameworks/GoMicro/LogInjection.expected |
Updates parameter SSA location. |
go/ql/test/library-tests/semmle/go/frameworks/GoMicro/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/library-tests/semmle/go/frameworks/GoKit/main.go |
Relocates endpoint annotations. |
go/ql/test/library-tests/semmle/go/frameworks/Gin/Gin.expected |
Updates extraction-node labels. |
go/ql/test/library-tests/semmle/go/frameworks/Gin/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/fasthttp.go |
Updates inline source expectations. |
go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/library-tests/semmle/go/frameworks/Echo/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference label. |
go/ql/test/library-tests/semmle/go/frameworks/Chi/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference label. |
go/ql/test/library-tests/semmle/go/frameworks/Beego/test.go |
Relocates source annotation. |
go/ql/test/library-tests/semmle/go/frameworks/Beego/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/library-tests/semmle/go/frameworks/Afero/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference label. |
go/ql/test/library-tests/semmle/go/dataflow/VarArgs/CONSISTENCY/DataFlowConsistency.expected |
Removes resolved consistency failure. |
go/ql/test/library-tests/semmle/go/dataflow/ThreatModels/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference label. |
go/ql/test/library-tests/semmle/go/dataflow/SSA/VarUses.expected |
Updates result-read nodes. |
go/ql/test/library-tests/semmle/go/dataflow/SliceExpressions/CONSISTENCY/CfgConsistency.expected |
Adds CFG consistency baseline. |
go/ql/test/library-tests/semmle/go/dataflow/ReadsAndWrites/writesField.expected |
Updates field-write instructions. |
go/ql/test/library-tests/semmle/go/dataflow/ReadsAndWrites/writesElement.expected |
Updates element-write instructions. |
go/ql/test/library-tests/semmle/go/dataflow/ReadsAndWrites/readsMethod.expected |
Updates implicit dereference label. |
go/ql/test/library-tests/semmle/go/dataflow/ReadsAndWrites/readsField.expected |
Updates implicit dereference label. |
go/ql/test/library-tests/semmle/go/dataflow/ReadsAndWrites/readsElement.expected |
Updates implicit dereference label. |
go/ql/test/library-tests/semmle/go/dataflow/PostUpdateNodes/test.expected |
Updates post-update dereference labels. |
go/ql/test/library-tests/semmle/go/dataflow/Nodes/resultParameters.go |
Relocates result-node annotations. |
go/ql/test/library-tests/semmle/go/dataflow/Nodes/ResultNode.expected |
Updates result-read nodes. |
go/ql/test/library-tests/semmle/go/dataflow/Nodes/CallNode_getResult_int.expected |
Updates extraction-node labels. |
go/ql/test/library-tests/semmle/go/dataflow/Nodes/BinaryOperationNodes.expected |
Updates compound-assignment label. |
go/ql/test/library-tests/semmle/go/dataflow/HiddenNodes/test.expected |
Updates extraction-node labels. |
go/ql/test/library-tests/semmle/go/dataflow/GlobalValueNumbering/GlobalValueNumber.expected |
Updates CFG instruction locations. |
go/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionOutput_isResult_int.expected |
Updates result extraction labels. |
go/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionOutput_getExitNode.expected |
Updates output exit nodes. |
go/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionOutput_getEntryNode.expected |
Updates zero-init nodes. |
go/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionInput_getExitNode.expected |
Updates parameter-init nodes. |
go/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionInput_getEntryNode.expected |
Updates SSA source ranges. |
go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/steps.expected |
Updates extraction-node labels. |
go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/srcs.expected |
Updates source-node locations. |
go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/steps.expected |
Updates extraction-node labels. |
go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/srcs.expected |
Updates source-node locations. |
go/ql/test/library-tests/semmle/go/dataflow/DefaultTaintSanitizer/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/NoretFunctions.expected |
Updates normal-return classification. |
go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/CONSISTENCY/CfgConsistency.expected |
Adds CFG consistency baseline. |
go/ql/test/library-tests/semmle/go/concepts/Regexp/RegexpPattern.expected |
Updates extraction and SSA nodes. |
go/ql/test/library-tests/semmle/go/concepts/HTTP/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/experimental/Unsafe/WrongUsageOfUnsafe.expected |
Updates SSA location. |
go/ql/test/experimental/Unsafe/CONSISTENCY/CfgConsistency.expected |
Adds CFG consistency baseline. |
go/ql/test/experimental/InconsistentCode/CONSISTENCY/CfgConsistency.expected |
Adds defer-loop CFG baseline. |
go/ql/test/experimental/frameworks/CleverGo/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference label. |
go/ql/test/experimental/CWE-942/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/experimental/CWE-918/SSRF.expected |
Updates dereference and extraction labels. |
go/ql/test/experimental/CWE-918/CONSISTENCY/DataFlowConsistency.expected |
Updates consistency output. |
go/ql/test/experimental/CWE-840/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/experimental/CWE-807/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/experimental/CWE-74/DsnInjectionLocal.expected |
Updates dereference labels. |
go/ql/test/experimental/CWE-369/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/experimental/CWE-321-V2/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/experimental/CWE-287/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/experimental/CWE-285/PamAuthBypass.expected |
Updates extraction-node label. |
go/ql/test/experimental/CWE-203/CONSISTENCY/DataFlowConsistency.expected |
Updates dereference labels. |
go/ql/test/example-tests/snippets/varwrite.expected |
Updates assignment label. |
go/ql/test/example-tests/snippets/typeinfo.expected |
Updates parameter-init nodes. |
go/ql/test/example-tests/snippets/fieldwrite.expected |
Updates assignment label. |
go/ql/src/RedundantCode/UnreachableStatement.ql |
Reworks unreachable-statement detection. |
go/ql/src/RedundantCode/DeadStoreOfLocal.ql |
Uses shared parameter initialization. |
go/ql/src/experimental/IntegerOverflow/RangeAnalysis.qll |
Adapts increment/decrement analysis. |
go/ql/lib/upgrades/b1341734d6870b105e5c9d168ce7dec25d7f72d0/upgrade.properties |
Declares range-element schema upgrade. |
go/ql/lib/semmle/go/StringOps.qll |
Handles omitted slice lower bounds. |
go/ql/lib/semmle/go/Stmt.qll |
Adds the range-element AST API. |
go/ql/lib/semmle/go/Scopes.qll |
Adds no-normal-return modeling hook. |
go/ql/lib/semmle/go/PrintAst.qll |
Makes the local overlay optional. |
go/ql/lib/semmle/go/frameworks/Zap.qll |
Migrates no-return model. |
go/ql/lib/semmle/go/frameworks/stdlib/Os.qll |
Migrates os.Exit model. |
go/ql/lib/semmle/go/frameworks/stdlib/Log.qll |
Migrates fatal-log model. |
go/ql/lib/semmle/go/frameworks/Revel.qll |
Adapts implicit field-read traversal. |
go/ql/lib/semmle/go/frameworks/Logrus.qll |
Migrates fatal/panic models. |
go/ql/lib/semmle/go/frameworks/Glog.qll |
Migrates fatal/exit models. |
go/ql/lib/semmle/go/Expr.qll |
Synthesizes key-value expression types. |
go/ql/lib/semmle/go/dataflow/SsaImpl.qll |
Connects SSA to the shared CFG. |
go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll |
Refines switch-edge filtering. |
go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll |
Adapts reachable and compound nodes. |
go/ql/lib/semmle/go/dataflow/GlobalValueNumbering.qll |
Anchors side-effect lookup to CFG entry. |
go/ql/lib/semmle/go/controlflow/BasicBlocks.qll |
Replaces bespoke basic blocks. |
go/ql/lib/semmle/go/Concepts.qll |
Migrates heuristic fatal logging model. |
go/ql/lib/printCfg.ql |
Adds the Go CFG viewer query. |
go/ql/lib/go.dbscheme |
Adds @rangeelementexpr. |
go/ql/consistency-queries/CfgConsistency.ql |
Enables shared CFG consistency checks. |
go/extractor/extractor.go |
Extracts synthesized range-element nodes. |
go/extractor/dbscheme/tables.go |
Registers the new expression kind. |
go/downgrades/23f2f56b5d3a846b4f73e3fa62510e36f934fb46/upgrade.properties |
Configures downgrade transforms. |
go/downgrades/23f2f56b5d3a846b4f73e3fa62510e36f934fb46/has_location.ql |
Removes synthesized-node locations. |
go/downgrades/23f2f56b5d3a846b4f73e3fa62510e36f934fb46/exprs.ql |
Reparents range variables on downgrade. |
Review details
- Files reviewed: 191/192 changed files
- Comments generated: 3
- Review effort level: Medium
owen-mc
force-pushed
the
go/shared-cfg
branch
3 times, most recently
from
July 16, 2026 06:46
d220d5c to
ff0384e
Compare
aschackmull
reviewed
Aug 18, 2026
aschackmull
reviewed
Aug 18, 2026
aschackmull
reviewed
Aug 18, 2026
aschackmull
reviewed
Aug 18, 2026
aschackmull
reviewed
Aug 18, 2026
aschackmull
reviewed
Aug 18, 2026
aschackmull
reviewed
Aug 18, 2026
aschackmull
reviewed
Aug 18, 2026
aschackmull
reviewed
Aug 18, 2026
Comment on lines
+279
to
+283
| // Go nests each case clause's body statements under the clause rather | ||
| // than in a flat list, so we expose a flattened view in which every | ||
| // case clause is immediately followed by its own body statements. This | ||
| // lets the shared library compute the body of a case as the statements | ||
| // between it and the next clause. |
Contributor
There was a problem hiding this comment.
Right. The shared lib actually supports both AST setups, but it does expect just a single body AstNode when the case bodies are nested under the case clauses. But since Go appears to have a sequence of statements as the body of a case clause, then I guess this is the easiest.
Contributor
Author
There was a problem hiding this comment.
I suppose we could extract a block statement to make it fit in with the shared CFG library better. But the workaround in ql is not too bad.
aschackmull
reviewed
Aug 18, 2026
aschackmull
reviewed
Aug 18, 2026
aschackmull
reviewed
Aug 18, 2026
aschackmull
reviewed
Aug 18, 2026
In Go, sub-expressions of a constant expression are folded at compile time and never evaluated at runtime, so they shouldn't get evaluation nodes.
|
|
||
| predicate selectedSourceLine = selectedSourceLineAlias/0; | ||
|
|
||
| predicate selectedSourceColumn = selectedSourceColumnAlias/0; |
|
|
||
| predicate selectedSourceColumn = selectedSourceColumnAlias/0; | ||
|
|
||
| predicate cfgScopeSpan( |
Comment on lines
1167
to
+1186
| } | ||
|
|
||
| /** | ||
| * A synthesized node grouping the loop variables (key and value) bound by a | ||
| * `range` statement. | ||
| * | ||
| * This node acts as the single target of the destructuring performed on each | ||
| * iteration of the loop, so that a `range` statement can be modelled with a | ||
| * single loop-variable node in the same way as a `foreach` loop in other | ||
| * languages. It is present for every `range` statement, even when no loop |
| * the case matches. | ||
| * | ||
| * In the control-flow graph the individual case test expressions of a case | ||
| * clause all funnel into a single "matched" node for the clause, from which |
|
|
||
| import go | ||
| private import ControlFlowGraphImpl | ||
| private import codeql.controlflow.SuccessorType |
aschackmull
reviewed
Sep 1, 2026
| * | ||
| * The default implementation does not override any edges. | ||
| */ | ||
| default predicate overridesAbruptCompletionEdge( |
Contributor
There was a problem hiding this comment.
We should be able to get rid of this + additionalSuccessor below. Here's how I imagine doing this: Add predicates mayPanic(AstNode) and mayDefer(AstNode) that are closed under getParent. Then add an additional node, "catch-panic" (or something like that), at ast nodes n for which mayPanic(n) and mayDefer(n.getParent()) holds. Then use this additional node in endAbruptCompletion such that panics are caught early enough in the ast that we still have maximal information about the defers that may be in scope. And doing it this way will ensure that we have an exception edge with the proper label. Then you can simply add an edge from this additional node to the proper place in the defer chain.
Returns can be handled in the same way.
aschackmull
reviewed
Sep 1, 2026
| * an edge from its "before" node, so the presence of an explicit step | ||
| * cannot itself suppress the default control flow. | ||
| */ | ||
| default predicate overridesDefaultControlFlow(AstNode ast) { none() } |
Contributor
There was a problem hiding this comment.
This hook should be unnecessary. It's currently the case that default control flow for ast gets overridden if a step originating at isBefore(ast) is provided.
In this case we should merely reconsider the CFG for select+send+receive slightly. The one that's provided currently skips over certain isBefore nodes, but there's really no good reason for doing that - adding isBefore(send) just before isIn(send) should e.g. fix things nicely for SendStmt.
aschackmull
reviewed
Sep 1, 2026
| * The default implementation preserves the existing behaviour: an | ||
| * explicit step from the "before" node suppresses all default steps. | ||
| */ | ||
| default predicate preservesDefaultControlFlow(AstNode ast) { none() } |
Contributor
There was a problem hiding this comment.
This + the below overridesDefaultControlFlowStep should also be unnecessary. If you're using a non-default CFG for some ast node then just specify it all instead of relying on a Franken-merge with parts of the default steps.
aschackmull
reviewed
Sep 1, 2026
| @@ -1819,11 +1898,25 @@ module Make0<LocationSig Location, AstSig<Location> Ast> { | |||
| not exists(getRankedCaseCfgOrder(switch, _)) and firstCase.isAfter(switch) | |||
Contributor
There was a problem hiding this comment.
I prefer adding and not simpleLeafNode(switch) here instead of disallowing switches to be simple leaf nodes.
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 PR migrates the Go control-flow graph (CFG) from its bespoke, Go-specific implementation to the shared CFG library. Broadly speaking, the commits are in these groups:
toStringandgetLocationpredicates for many CFG classes and accepts all test changes.incdec-rhs/compound-rhs, foldingzero-initand write nodes together, mergingresult-initintoresult-zero-init, dropping implicit slice-bound nodes, and no longer emitting CFG nodes for subexpressions of constant expressions.I've tried to always make it so that a commit contains any test changes which it causes, so their effect can easily be seen while reviewing.
Note that
additionalNodesis quite a lot bigger than in other languages, like java and C#. The reason for the disparity is architectural: Go's dataflow nodes are CFG-instruction-based (MkInstructionNode), not AST-based like Java/C#'sTExprNode. Switching to AST-keyed expr nodes (or mapping values via injects) would make Go's CFG implementation more like java and C#, but that's a larger IR redesign we'd do separately.