-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Go: Use shared basic block lib #21224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
6fa0d2e to
497f350
Compare
497f350 to
e1cf0a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 <175728472+Copilot@users.noreply.github.com>
|
DCA doesn't show any changes. |
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.