Skip to content

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)

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

@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!

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