Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
owen-mc
force-pushed
the
go/use-shared-basic-block-lib
branch
2 times, most recently
from
6fa0d2e to
497f350
Compare
owen-mc
force-pushed
the
go/use-shared-basic-block-lib
branch
from
497f350 to
e1cf0a1
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request refactors the Go language's basic block implementation to use a shared basic blocks library from the CodeQL standard library, resulting in cleaner code and better consistency across languages.
Changes:
- Migrated
BasicBlockclass implementation from custom code to the sharedcodeql.controlflow.BasicBlocklibrary - Updated API across the codebase: renamed
getRoot()togetScope(), added successor type parameters togetAPredecessor()andgetASuccessor(), and replacedinDominanceFrontierOf()withinDominanceFrontier()with swapped arguments - Added supporting infrastructure including the
ControlFlowNodetype alias andgetOutcome()method forConditionGuardNode
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| go/ql/lib/semmle/go/controlflow/BasicBlocks.qll | Replaced custom basic block implementation with shared library integration through Input module and BbImpl instantiation |
| go/ql/lib/semmle/go/dataflow/SsaImpl.qll | Updated API calls: getRoot() to getScope(), added _ parameter to successor/predecessor calls, and swapped inDominanceFrontierOf() arguments |
| go/ql/lib/semmle/go/dataflow/SSA.qll | Updated getRoot() to getScope() and added _ parameter to getAPredecessor() |
| go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll | Added getOutcome() method to ConditionGuardNode and ControlFlowNode type alias to support shared library integration |
| go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql | Updated getAPredecessor() call to include _ parameter |
| go/ql/lib/qlpack.yml | Added dependency on codeql/controlflow library |
| go/ql/lib/change-notes/2026-01-28-shared-basic-block-library.md | Documented breaking changes from this refactoring |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
go/ql/lib/change-notes/2026-01-28-shared-basic-block-library.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
Contributor
Author
|
DCA doesn't show any changes. |
hvitved
approved these changes
Feb 2, 2026
| class EntryBasicBlock = BbImpl::EntryBasicBlock; | ||
|
|
||
| cached | ||
| private predicate reachableBB(BasicBlock bb) { |
Contributor
There was a problem hiding this comment.
Note that now this predicate will be in a separate caching stage from the predicates cached inside the shared library. You can add an artificial dependency on this predicate in the input module to collapse them, if needed.
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
2 participants
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.
Use the shared library to instantiate basic blocks. This leads to a small number of breaking changes to member predicate names, as detailed in the change note.
I wasn't sure if
ReachableBasicBlockandReachableJoinBlockare really needed any more. Their main purpose seems to have been to avoid unnecessary calculations in the dominance frontier calculation and some SSA calculations. I will leave them for now - I imagine they'll disappear when we adopt the shared SSA library at some point in the future.