Repository navigation
Share: Handle exhaustive switches in the shared CFG library - #23026
Merged
Merged
Conversation
paldepind
changed the title
switches in the shared CFG library
paldepind
force-pushed
the
unified/match-all
branch
from
November 5, 2026 10:53
8e5ff7b to
85f7b88
Compare
paldepind
changed the title
switches in the shared CFG library
paldepind
changed the title
switches in the shared CFG library
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4a9b52f8-d07b-4692-8fb9-409f0efd64f2
Add an `isExhaustive` input hook and treat the final case of an exhaustive switch as match-all. Default to non-exhaustive to preserve existing behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4a9b52f8-d07b-4692-8fb9-409f0efd64f2
Use the shared exhaustive-switch handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4a9b52f8-d07b-4692-8fb9-409f0efd64f2
Declare exhaustive Java switch constructs through `isExhaustive`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4a9b52f8-d07b-4692-8fb9-409f0efd64f2
Keep only language-specific switch-arm match-all logic, since the shared control-flow graph now handles default cases directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4a9b52f8-d07b-4692-8fb9-409f0efd64f2
paldepind
force-pushed
the
unified/match-all
branch
from
November 5, 2026 11:53
85f7b88 to
d383b86
Compare
paldepind
marked this pull request as ready for review
November 5, 2026 11:56
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical exhaustiveness inference errors can make default arms unreachable and remove valid Swift switch continuations, potentially hiding downstream taint flows.
2 open issues
What changed in this PR
Adds exhaustive-switch support to the shared control-flow graph library, allowing Unified’s Swift analysis to remove impossible fall-through paths while adapting the Java and C# integrations.
Changes:
- Introduce
isExhaustiveand centralize default-case handling in the shared CFG. - Enable exhaustive Swift switches and update Java and C# matching hooks.
- Add Swift Boolean and enum switch tests and refresh expected CFG results.
| File | Description |
|---|---|
unified/ql/test/library-tests/controlflow/cfg.swift |
Add exhaustive Boolean and enum switch tests, including a guarded case. |
unified/ql/test/library-tests/controlflow/cfg.expected |
Record case edges and unreachable post-switch returns. |
unified/ql/test/library-tests/controlflow/basicblock-slices.expected |
Update basic-block expectations for the new tests. |
unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll |
Mark Swift switches as exhaustive. |
shared/controlflow/codeql/controlflow/ControlFlowGraph.qll |
Add the exhaustiveness hook and shared match-all handling. |
java/ql/lib/semmle/code/java/ControlFlowGraph.qll |
Separate switch exhaustiveness from match-all patterns. |
csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowGraph.qll |
Delegate default-case handling to the shared library. |
Review effort: Max (auto)
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The switch-exhaustiveness guard misses default cases outside the AST-last position, incorrectly treating a later case as match-all and making the default arm unreachable in the CFG.
2 open issues
1 resolved since last review
Review effort: Max (auto)
paldepind
force-pushed
the
unified/match-all
branch
from
November 5, 2026 13:03
c6cd4b6 to
8459473
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A Java regression test with regenerated expectations is still needed for a switch expression whose default precedes another case, since the current tests do not cover that ordering.
1 open issue
2 resolved since last review
Review effort: Max (auto)
| // Default cases are always considered last in control flow order, so | ||
| // exhaustiveness does not apply to the last case if a default case | ||
| // exists. | ||
| not switch.getCase(_) instanceof DefaultCase and |
Contributor
Author
|
Thanks for the review. |
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:
switchstatements/expressions.DefaultCasecase handling from the defaultmatchAllimplementation. This makes it possible to customizematchAllwithout having to re-implementDefaultCasehandling.switchhandling. This improves the CFG for Swift.