Shared: Add a shared SuccessorType implementation #20300
Conversation
| @@ -882,7 +881,7 @@ | |||
| /** Gets the type of the exception being thrown. */ | |||
| ExceptionClass getExceptionClass() { result = ec } | |||
|
|
|||
| override ExceptionSuccessor getAMatchingSuccessorType() { result.getExceptionClass() = ec } | |||
| override ExceptionSuccessor getAMatchingSuccessorType() { any() } | |||
Check warning
Code scanning / CodeQL
Override with unmentioned parameter Warning
| @@ -431,7 +430,7 @@ | |||
| this = TNestedCompletion(_, TRaiseCompletion(), _) | |||
| } | |||
|
|
|||
| override RaiseSuccessor getAMatchingSuccessorType() { any() } | |||
| override ExceptionSuccessor getAMatchingSuccessorType() { any() } | |||
Check warning
Code scanning / CodeQL
Override with unmentioned parameter Warning
ce85279 to
75ea5f2
Compare
| @@ -468,7 +467,7 @@ | |||
|
|
|||
| FallthroughCompletion() { this = TFallthroughCompletion(dest) } | |||
|
|
|||
| override FallthroughSuccessor getAMatchingSuccessorType() { any() } | |||
| override DirectSuccessor getAMatchingSuccessorType() { any() } | |||
Check warning
Code scanning / CodeQL
Override with unmentioned parameter Warning
Mostly toString changes, and a slight change to splitting in C#.
d9a59a2 to
01fcc41
Compare
| @@ -859,7 +858,7 @@ | |||
| /** Gets the label of the `goto` completion. */ | |||
| string getLabel() { result = label } | |||
|
|
|||
| override GotoSuccessor getAMatchingSuccessorType() { result.getLabel() = label } | |||
| override GotoSuccessor getAMatchingSuccessorType() { any() } | |||
Check warning
Code scanning / CodeQL
Override with unmentioned parameter Warning
| @@ -377,7 +376,7 @@ | |||
| this = TNestedCompletion(_, TNextCompletion(), _) | |||
| } | |||
|
|
|||
| override NextSuccessor getAMatchingSuccessorType() { any() } | |||
| override ContinueSuccessor getAMatchingSuccessorType() { any() } | |||
Check warning
Code scanning / CodeQL
Override with unmentioned parameter Warning
01fcc41 to
0df7d6f
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds a shared implementation of the SuccessorType class used as edge labels in control flow graphs (CFGs). The implementation centralizes successor type definitions in the shared/controlflow module and updates all languages to use this shared code instead of maintaining language-specific implementations.
Key changes include:
- Adding a comprehensive shared
SuccessorTypeimplementation with a well-defined hierarchy - Removing language-specific successor type implementations
- Updating all languages to import and use the shared successor types
Reviewed Changes
Copilot reviewed 64 out of 68 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| shared/controlflow/codeql/controlflow/SuccessorType.qll | New shared successor type implementation with comprehensive hierarchy |
| shared/controlflow/codeql/controlflow/Guards.qll | Updated to import shared successor types |
| shared/controlflow/codeql/controlflow/Cfg.qll | Updated to use shared successor types and removed language-specific definitions |
| shared/controlflow/codeql/controlflow/BasicBlock.qll | Updated to import shared successor types |
| swift/ql/lib/codeql/swift/security/PathInjectionExtensions.qll | Updated to use shared BooleanSuccessor type |
| swift/ql/lib/codeql/swift/controlflow/* | Removed Swift-specific successor types and updated to use shared implementation |
| rust/ql/lib/codeql/rust/controlflow/* | Removed Rust-specific successor types and updated to use shared implementation |
| ruby/ql/lib/codeql/ruby/controlflow/* | Removed Ruby-specific successor types and updated to use shared implementation |
| java/ql/lib/semmle/code/java/controlflow/SuccessorType.qll | Removed Java-specific successor type implementation |
| csharp/ql/test/library-tests/goto/Goto1.expected | Updated test expectations for goto successor label changes |
| ruby/ql/test/library-tests/controlflow/graph/*.expected | Updated test expectations for successor type label changes |
Comments suppressed due to low confidence (1)
swift/ql/lib/codeql/swift/controlflow/internal/Completion.qll:1
- The change from
MatchSuccessortoMatchingSuccessorsuggests a naming inconsistency between the old Swift implementation and the new shared implementation. Ensure all references are updated consistently.
/**
This adds a shared implementation of the
SuccessorTypeclass that's used as edge labels in our CFGs. I've also updated all languages and all shared libraries to use this new shared code.Commit-by-commit review is encouraged.
A few notes:
SuccessorTypeis now a shared non-parameterized implementation. This means that it's not using language-specific details like exception type or goto-labels.SuccessorType, so the change has a slight semantic impact here. Initially I tried a version that bunched all of the jump successors together, but that gave a single new FP in C# nullness, so I choose to have a more detailed distinction between different jump successors.successorTypeIsConditionpredicate in a few places in order to not disrupt basic block boundaries too much. Streamlining this across languages is left as a potential follow-up consideration.