Cfg: Change AST/CFG for CatchClauses to use a pattern.#22017
Draft
aschackmull wants to merge 1 commit into
Draft
Cfg: Change AST/CFG for CatchClauses to use a pattern.#22017aschackmull wants to merge 1 commit into
aschackmull wants to merge 1 commit into
Conversation
aschackmull
added
the
no-change-note-required
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request updates the shared control-flow graph (CFG) construction to treat CatchClause exception handling as a pattern match with match-completion, aligning the shared AstSig with other languages (Ruby/Python) and the upcoming unified AST model, and adding support for catch clauses with missing bodies.
Changes:
- Extend
AstSig.CatchClausewithgetPattern()and update catch-all semantics to be based on the absence of a pattern. - Update shared CFG construction to route evaluation through catch patterns (and to handle catch clauses that have no body via an additional node/tag).
- Adapt Java and C# language-specific
CatchClausemappings so the variable declaration is modeled as the catch pattern.
Show a summary per file
| File | Description |
|---|---|
| shared/controlflow/codeql/controlflow/ControlFlowGraph.qll | Updates CatchClause AstSig and shared CFG building to use a pattern/match-completion model and handle missing catch bodies. |
| java/ql/lib/semmle/code/java/ControlFlowGraph.qll | Maps Java catch variable declaration to CatchClause.getPattern() and disables a separate getVariable() node. |
| csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraph.qll | Maps C# catch variable declaration expression to CatchClause.getPattern() and disables a separate getVariable() node. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 1
Comment on lines
+237
to
241
| * Gets the variable declared by this catch clause, if any. | ||
| * | ||
| * Some languages include the variable binding as part of the pattern. | ||
| */ | ||
| AstNode getVariable(); |
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 updates the AstSig and corresponding CFG for CatchClauses to use a pattern with a match-completion. This increases alignment with both Ruby, Python, and in particular the upcoming unified AST. Support for missing catch clause bodies is also added.
For Java and C#, the variable declaration is simply seen as the pattern that may match or not as that's the part containing the type test. This is expected to also be the general pattern in the unified AST while the CFG lib retains the
getVariablemember in order to more easily support Ruby and Python (in which the type test and the variable binding are independent child nodes of the catch).