Split states for is not patterns

src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs

@@ -51,55 +51,49 @@ private BoundExpression MakeIsPatternExpression(
             // Note that these labels are for the convenience of the compilation of patterns, and are not necessarily emitted into the lowered code.
             LabelSymbol whenTrueLabel = new GeneratedLabelSymbol("isPatternSuccess");
             LabelSymbol whenFalseLabel = new GeneratedLabelSymbol("isPatternFailure");
-
-            bool negated = pattern.IsNegated(out var innerPattern);
             BoundDecisionDag decisionDag = DecisionDagBuilder.CreateDecisionDagForIsPattern(
-                this.Compilation, pattern.Syntax, expression, innerPattern, whenTrueLabel: whenTrueLabel, whenFalseLabel: whenFalseLabel, diagnostics);
-
-            if (!hasErrors && getConstantResult(decisionDag, negated, whenTrueLabel, whenFalseLabel) is { } constantResult)
+                this.Compilation, pattern.Syntax, expression, pattern, whenTrueLabel: whenTrueLabel, whenFalseLabel: whenFalseLabel, diagnostics);
+            if (!hasErrors && !decisionDag.ReachableLabels.Contains(whenTrueLabel))
             {
-                if (!constantResult)
-                {
-                    Debug.Assert(expression.Type is object);
-                    diagnostics.Add(ErrorCode.ERR_IsPatternImpossible, node.Location, expression.Type);
-                    hasErrors = true;
-                }
-                else
+                Debug.Assert(expression.Type is object);
+                diagnostics.Add(ErrorCode.ERR_IsPatternImpossible, node.Location, expression.Type);
+                hasErrors = true;
+            }
+            else if (!hasErrors && !decisionDag.ReachableLabels.Contains(whenFalseLabel))
+            {
+                switch (pattern)
                 {
-                    switch (pattern)
-                    {
-                        case BoundConstantPattern _:
-                        case BoundITuplePattern _:
-                            // these patterns can fail in practice
-                            throw ExceptionUtilities.Unreachable;
-                        case BoundRelationalPattern _:
-                        case BoundTypePattern _:
-                        case BoundNegatedPattern _:
-                        case BoundBinaryPattern _:
-                            Debug.Assert(expression.Type is object);
-                            diagnostics.Add(ErrorCode.WRN_IsPatternAlways, node.Location, expression.Type);
-                            break;
-                        case BoundDiscardPattern _:
-                            // we do not give a warning on this because it is an existing scenario, and it should
-                            // have been obvious in source that it would always match.
-                            break;
-                        case BoundDeclarationPattern _:
-                        case BoundRecursivePattern _:
-                            // We do not give a warning on these because people do this to give a name to a value
-                            break;
-                    }
+                    case BoundConstantPattern _:
+                    case BoundITuplePattern _:
+                        // these patterns can fail in practice
+                        throw ExceptionUtilities.Unreachable;
+                    case BoundRelationalPattern _:
+                    case BoundTypePattern _:
+                    case BoundNegatedPattern _:
+                    case BoundBinaryPattern _:
+                        Debug.Assert(expression.Type is object);
+                        diagnostics.Add(ErrorCode.WRN_IsPatternAlways, node.Location, expression.Type);
+                        break;
+                    case BoundDiscardPattern _:
+                        // we do not give a warning on this because it is an existing scenario, and it should
+                        // have been obvious in source that it would always match.
+                        break;
+                    case BoundDeclarationPattern _:
+                    case BoundRecursivePattern _:
+                        // We do not give a warning on these because people do this to give a name to a value
+                        break;
                 }
             }
             else if (expression.ConstantValue != null)
             {
                 decisionDag = decisionDag.SimplifyDecisionDagIfConstantInput(expression);
-                if (!hasErrors && getConstantResult(decisionDag, negated, whenTrueLabel, whenFalseLabel) is { } simplifiedResult)
+                if (!hasErrors)
                 {
-                    if (!simplifiedResult)
+                    if (!decisionDag.ReachableLabels.Contains(whenTrueLabel))
                     {
                         diagnostics.Add(ErrorCode.WRN_GivenExpressionNeverMatchesPattern, node.Location);
                     }
-                    else
+                    else if (!decisionDag.ReachableLabels.Contains(whenFalseLabel))
                     {
                         switch (pattern)
                         {
@@ -118,23 +112,8 @@ private BoundExpression MakeIsPatternExpression(
                 }
             }
 
-            // decisionDag, whenTrueLabel, and whenFalseLabel represent the decision DAG for the inner pattern,
-            // after removing any outer 'not's, so consumers will need to compensate for negated patterns.
             return new BoundIsPatternExpression(
-                node, expression, pattern, negated, decisionDag, whenTrueLabel: whenTrueLabel, whenFalseLabel: whenFalseLabel, boolType, hasErrors);
-
-            static bool? getConstantResult(BoundDecisionDag decisionDag, bool negated, LabelSymbol whenTrueLabel, LabelSymbol whenFalseLabel)
-            {
-                if (!decisionDag.ReachableLabels.Contains(whenTrueLabel))
-                {
-                    return negated;
-                }
-                else if (!decisionDag.ReachableLabels.Contains(whenFalseLabel))
-                {
-                    return !negated;
-                }
-                return null;
-            }
+                node, expression, pattern, decisionDag, whenTrueLabel: whenTrueLabel, whenFalseLabel: whenFalseLabel, boolType, hasErrors);
         }
 
         private BoundExpression BindSwitchExpression(SwitchExpressionSyntax node, BindingDiagnosticBag diagnostics)

src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs

@@ -112,14 +112,13 @@ public static BoundDecisionDag CreateDecisionDagForIsPattern(
             return builder.CreateDecisionDagForIsPattern(syntax, inputExpression, pattern, whenTrueLabel);
         }
 
-        private BoundDecisionDag CreateDecisionDagForIsPattern(
-            SyntaxNode syntax,
+        private BoundDecisionDag CreateDecisionDagForIsPattern(SyntaxNode syntax,
             BoundExpression inputExpression,
             BoundPattern pattern,
             LabelSymbol whenTrueLabel)
         {
             var rootIdentifier = BoundDagTemp.ForOriginalInput(inputExpression);
-            return MakeBoundDecisionDag(syntax, ImmutableArray.Create(MakeTestsForPattern(index: 1, pattern.Syntax, rootIdentifier, pattern, whenClause: null, whenTrueLabel)));
+            return MakeBoundDecisionDag(syntax, MakeTestsForIsPattern(pattern.Syntax, rootIdentifier, pattern, whenTrueLabel));
         }
 
         private BoundDecisionDag CreateDecisionDagForSwitchStatement(
@@ -176,6 +175,25 @@ private StateForCase MakeTestsForPattern(
             return new StateForCase(index, syntax, tests, bindings, whenClause, label);
         }
 
+        private ImmutableArray<StateForCase> MakeTestsForIsPattern(
+            SyntaxNode syntax,
+            BoundDagTemp input,
+            BoundPattern pattern,
+            LabelSymbol whenTrueLabel)
+        {
+            Tests tests = MakeAndSimplifyTestsAndBindings(input, pattern, out ImmutableArray<BoundPatternBinding> bindings);
+            if (pattern.Kind != BoundKind.NegatedPattern)
+            {
+                return ImmutableArray.Create(new StateForCase(1, syntax, tests, bindings, whenClause: null, whenTrueLabel));
+            }
+            else
+            {
+                return ImmutableArray.Create(
+                    new StateForCase(1, syntax, tests, ImmutableArray<BoundPatternBinding>.Empty, whenClause: null, whenTrueLabel),
+                    new StateForCase(2, syntax, Tests.True.Instance, bindings, whenClause: null, _defaultLabel, required: true));
+            }
+        }
+
         private Tests MakeAndSimplifyTestsAndBindings(
             BoundDagTemp input,
             BoundPattern pattern,
@@ -844,24 +862,24 @@ private void ComputeBoundDecisionDagNodes(DecisionDag decisionDag, BoundLeafDeci
                     if (first.IsFullyMatched)
                     {
                         // there is no when clause we need to evaluate
-                        state.Dag = finalState(first.Syntax, first.CaseLabel, first.Bindings);
+                        state.Dag = finalState(first);
                     }
                     else
                     {
                         RoslynDebug.Assert(state.TrueBranch == null);
                         RoslynDebug.Assert(state.FalseBranch is { });
 
                         // The final state here does not need bindings, as they will be performed before evaluating the when clause (see below)
-                        BoundDecisionDagNode whenTrue = finalState(first.Syntax, first.CaseLabel, default);
+                        BoundDecisionDagNode whenTrue = finalState(first, performBindings: false);
                         BoundDecisionDagNode? whenFalse = state.FalseBranch.Dag;
                         RoslynDebug.Assert(whenFalse is { });
-                        state.Dag = uniqifyDagNode(new BoundWhenDecisionDagNode(first.Syntax, first.Bindings, first.WhenClause, whenTrue, whenFalse));
+                        state.Dag = uniqifyDagNode(new BoundWhenDecisionDagNode(first.Syntax, first.Bindings, first.WhenClause, whenTrue, whenFalse, required: false));
                     }
 
-                    BoundDecisionDagNode finalState(SyntaxNode syntax, LabelSymbol label, ImmutableArray<BoundPatternBinding> bindings)
+                    BoundDecisionDagNode finalState(StateForCase state, bool performBindings = true)
                     {
-                        BoundDecisionDagNode final = uniqifyDagNode(new BoundLeafDecisionDagNode(syntax, label));
-                        return bindings.IsDefaultOrEmpty ? final : uniqifyDagNode(new BoundWhenDecisionDagNode(syntax, bindings, null, final, null));
+                        BoundDecisionDagNode final = uniqifyDagNode(new BoundLeafDecisionDagNode(state.Syntax, state.CaseLabel));
+                        return !performBindings || state.Bindings.IsDefaultOrEmpty ? final : uniqifyDagNode(new BoundWhenDecisionDagNode(state.Syntax, state.Bindings, null, final, null, state.Required));
                     }
                 }
                 else
@@ -1023,9 +1041,9 @@ private static ImmutableArray<StateForCase> RemoveEvaluation(ImmutableArray<Stat
                 else
                 {
                     builder.Add(new StateForCase(
-                        Index: stateForCase.Index, Syntax: stateForCase.Syntax,
-                        RemainingTests: remainingTests,
-                        Bindings: stateForCase.Bindings, WhenClause: stateForCase.WhenClause, CaseLabel: stateForCase.CaseLabel));
+                        stateForCase.Index, stateForCase.Syntax,
+                        remainingTests,
+                        stateForCase.Bindings, stateForCase.WhenClause, stateForCase.CaseLabel, stateForCase.Required));
                 }
             }
 
@@ -1574,20 +1592,23 @@ private sealed class StateForCase
             public readonly ImmutableArray<BoundPatternBinding> Bindings;
             public readonly BoundExpression? WhenClause;
             public readonly LabelSymbol CaseLabel;
+            public readonly bool Required;
             public StateForCase(
-                int Index,
-                SyntaxNode Syntax,
-                Tests RemainingTests,
-                ImmutableArray<BoundPatternBinding> Bindings,
-                BoundExpression? WhenClause,
-                LabelSymbol CaseLabel)
+                int index,
+                SyntaxNode syntax,
+                Tests remainingTests,
+                ImmutableArray<BoundPatternBinding> bindings,
+                BoundExpression? whenClause,
+                LabelSymbol caseLabel,
+                bool required = false)
             {
-                this.Index = Index;
-                this.Syntax = Syntax;
-                this.RemainingTests = RemainingTests;
-                this.Bindings = Bindings;
-                this.WhenClause = WhenClause;
-                this.CaseLabel = CaseLabel;
+                this.Index = index;
+                this.Syntax = syntax;
+                this.RemainingTests = remainingTests;
+                this.Bindings = bindings;
+                this.WhenClause = whenClause;
+                this.CaseLabel = caseLabel;
+                this.Required = required;
             }
 
             /// <summary>

src/Compilers/CSharp/Portable/BoundTree/BoundDecisionDag.cs

@@ -124,7 +124,7 @@ public static BoundDecisionDagNode TrivialReplacement(BoundDecisionDagNode dag,
                 case BoundTestDecisionDagNode p:
                     return p.Update(p.Test, replacement(p.WhenTrue), replacement(p.WhenFalse));
                 case BoundWhenDecisionDagNode p:
-                    return p.Update(p.Bindings, p.WhenExpression, replacement(p.WhenTrue), (p.WhenFalse != null) ? replacement(p.WhenFalse) : null);
+                    return p.Update(p.Bindings, p.WhenExpression, replacement(p.WhenTrue), (p.WhenFalse != null) ? replacement(p.WhenFalse) : null, p.Required);
                 case BoundLeafDecisionDagNode p:
                     return p;
                 default:

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

@@ -1371,6 +1371,10 @@
     <Field Name="WhenExpression" Type="BoundExpression?"/>
     <Field Name="WhenTrue" Type="BoundDecisionDagNode" Null="disallow"/>
     <Field Name="WhenFalse" Type="BoundDecisionDagNode?"/>
+    <!--
+    Required is true when bindings are used in the opposite branch and cannot be optimized away.
+    -->
+    <Field Name="Required" Type="bool"/>
   </Node>
   <!-- This node is used to indicate a point in the decision dag where the decision has been completed. -->
   <Node Name="BoundLeafDecisionDagNode" Base="BoundDecisionDagNode">
@@ -2021,15 +2025,10 @@
     <Field Name="Alignment" Type="BoundExpression?"/>
     <Field Name="Format" Type="BoundLiteral?"/>
   </Node>
-
-    <!-- An 'is pattern'. The fields DecisionDag, WhenTrueLabel, and WhenFalseLabel represent the inner pattern
-         after removing any outer 'not's, so consumers (such as lowering and definite assignment of the local in
-         'is not Type t') will need to compensate for negated patterns. IsNegated is set if Pattern is the negated
-         form of the inner pattern represented by DecisionDag. -->
+
   <Node Name="BoundIsPatternExpression" Base="BoundExpression">
     <Field Name="Expression" Type="BoundExpression" Null="disallow"/>
     <Field Name="Pattern" Type="BoundPattern" Null="disallow"/>
-    <Field Name="IsNegated" Type="bool"/>
     <Field Name="DecisionDag" Type="BoundDecisionDag" Null="disallow" SkipInVisitor="true"/>
     <Field Name="WhenTrueLabel" Type="LabelSymbol" Null="disallow"/>
     <Field Name="WhenFalseLabel" Type="LabelSymbol" Null="disallow"/>

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs

@@ -962,8 +962,13 @@ public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node
             Debug.Assert(!IsConditionalState);
             VisitRvalue(node.Expression);
 
-            bool negated = node.Pattern.IsNegated(out var pattern);
-            Debug.Assert(negated == node.IsNegated);
+            BoundPattern pattern = node.Pattern;
+            bool negated = false;
+            while (pattern is BoundNegatedPattern p)
+            {
+                negated = !negated;
+                pattern = p.Negated;
+            }
 
             VisitPattern(pattern);
             var reachableLabels = node.DecisionDag.ReachableLabels;

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs

@@ -848,8 +848,8 @@ public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node
             var expressionState = ResultType;
             var state = PossiblyConditionalState.Create(this);
             var labelStateMap = LearnFromDecisionDag(node.Syntax, node.DecisionDag, node.Expression, expressionState, ref state);
-            var trueState = labelStateMap.TryGetValue(node.IsNegated ? node.WhenFalseLabel : node.WhenTrueLabel, out var s1) ? s1.state : UnreachableState();
-            var falseState = labelStateMap.TryGetValue(node.IsNegated ? node.WhenTrueLabel : node.WhenFalseLabel, out var s2) ? s2.state : UnreachableState();
+            var trueState = labelStateMap.TryGetValue(node.WhenTrueLabel, out var s1) ? s1.state : UnreachableState();
+            var falseState = labelStateMap.TryGetValue(node.WhenFalseLabel, out var s2) ? s2.state : UnreachableState();
             labelStateMap.Free();
             SetConditionalState(trueState, falseState);
             SetNotNullResult(node);