Skip to content
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

Add minimal binary pattern support for null checks in DFA #6877

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Aug 20, 2023

Fixes #6755

@mavasani I have added very very minimal handling for binary patterns.

This doesn't handle declaration patterns, etc etc.

I can try to go further with the implementation whenever I get time again :)

I know my implementation might not be good enough. So, feel free if you wanted to close this PR and have a completely different fix.

@Youssef1313 Youssef1313 requested a review from a team as a code owner August 20, 2023 16:52
@codecov
Copy link

codecov bot commented Aug 20, 2023

Codecov Report

Merging #6877 (b1d9dc2) into main (3587540) will decrease coverage by 0.01%.
Report is 3 commits behind head on main.
The diff coverage is 97.95%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6877      +/-   ##
==========================================
- Coverage   96.39%   96.39%   -0.01%     
==========================================
  Files        1403     1403              
  Lines      330977   331075      +98     
  Branches    10890    10900      +10     
==========================================
+ Hits       319055   319148      +93     
- Misses       9188     9191       +3     
- Partials     2734     2736       +2     

@@ -1612,6 +1612,25 @@ private void PerformPredicateAnalysisCore(IOperation operation, TAnalysisData ta
case OperationKind.BinaryPattern:
// These high level patterns should not be present in the lowered CFG: https://github.com/dotnet/roslyn/issues/47068
predicateValueKind = PredicateValueKind.Unknown;

// We special case common null check to reduce false positives. But this implementation for BinaryPattern is very incomplete.
if (FlowBranchConditionKind == ControlFlowConditionKind.WhenFalse)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice optimization! Generic flow analysis handling for binary pattern operations is going to be non-trivial as there are sort of conditional branches with the and and or operators. I think this is a good starting point to handle the most important cases.

Copy link
Contributor

@mavasani mavasani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Youssef1313!

@mavasani mavasani merged commit bab9b10 into dotnet:main Aug 29, 2023
@Youssef1313 Youssef1313 deleted the dfa-binary-pattern branch August 29, 2023 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CA1062 stumbles when pattern-matching with []
2 participants