C#: Replace old Guards with the new shared implementation. #20665
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| private module ControlFlowInput implements | ||
| InputSig<Location, ControlFlow::Node, ControlFlow::BasicBlock> | ||
| { | ||
| private import csharp as CS |
Check warning
Code scanning / CodeQL
Names only differing by case Warning
| private module GuardsInput implements | ||
| SharedGuards::InputSig<Location, ControlFlow::Node, ControlFlow::BasicBlock> | ||
| { | ||
| private import csharp as CS |
Check warning
Code scanning / CodeQL
Names only differing by case Warning
aschackmull
force-pushed
the
csharp/guards-replace
branch
2 times, most recently
from
6b712ec to
5163620
Compare
aschackmull
force-pushed
the
csharp/guards-replace
branch
4 times, most recently
from
fe690df to
e1f16df
Compare
aschackmull
force-pushed
the
csharp/guards-replace
branch
from
e1f16df to
87d89fd
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances exception handling and guard analysis in the QL control flow library. The changes extend exception-like successor types to include exit successors and introduce additional implication steps for better guard reasoning.
Key changes:
- Extended exception handling to treat exit successors similarly to exception successors
- Added
trivialHasValuepredicate to filter out trivial guard conditions - Introduced
additionalImpliesStepextension point for custom implication logic - Updated test expectations to reflect improved null analysis
Reviewed Changes
Copilot reviewed 32 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| shared/controlflow/codeql/controlflow/Guards.qll | Core logic changes: added exceptionLike predicate, trivialHasValue helper, and extension point for implications |
| csharp/ql/test/query-tests/Nullness/*.expected | Updated test expectations showing improved null check detection |
| csharp/ql/test/query-tests/Nullness/*.ql | Simplified test queries to use new APIs |
| csharp/ql/test/query-tests/Nullness/Forwarding.cs | Test case updated with comment about expected false positive |
| csharp/ql/test/query-tests/Nullness/C.cs | Test case made more realistic by adding conditional assignment |
| csharp/ql/test/library-tests/controlflow/guards/*.ql | Removed deprecated test queries |
| csharp/ql/test/library-tests/controlflow/guards/*.expected | Updated expectations for new guard behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Dca looks reasonable, I think. Some new FPs, a few new TPs, and some FP removal. |
aschackmull
added
the
no-change-note-required
2 participants
This PR does two things: First, the old C# Guards library is replaced by the newly instantiated shared Guards library. Second, splitting on assertions is removed and replaced by an implication in the Guards library.
Commit-by-commit review is encouraged.