Split states for is not patterns

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

@@ -52,55 +52,47 @@ 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
+                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 _:
+                        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)
                         {
@@ -119,23 +111,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

@@ -109,17 +109,17 @@ public static BoundDecisionDag CreateDecisionDagForIsPattern(
             BindingDiagnosticBag diagnostics)
         {
             var builder = new DecisionDagBuilder(compilation, defaultLabel: whenFalseLabel, diagnostics);
-            return builder.CreateDecisionDagForIsPattern(syntax, inputExpression, pattern, whenTrueLabel);
+            return builder.CreateDecisionDagForIsPattern(syntax, inputExpression, pattern, whenTrueLabel, whenFalseLabel);
         }
 
-        private BoundDecisionDag CreateDecisionDagForIsPattern(
-            SyntaxNode syntax,
+        private BoundDecisionDag CreateDecisionDagForIsPattern(SyntaxNode syntax,
             BoundExpression inputExpression,
             BoundPattern pattern,
-            LabelSymbol whenTrueLabel)
+            LabelSymbol whenTrueLabel,
+            LabelSymbol whenFalseLabel)
         {
             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, whenFalseLabel));
         }
 
         private BoundDecisionDag CreateDecisionDagForSwitchStatement(
@@ -176,6 +176,24 @@ private StateForCase MakeTestsForPattern(
             return new StateForCase(index, syntax, tests, bindings, whenClause, label);
         }
 
+        private ImmutableArray<StateForCase> MakeTestsForIsPattern(
+            SyntaxNode syntax,
+            BoundDagTemp input,
+            BoundPattern pattern,
+            LabelSymbol whenTrueLabel,
+            LabelSymbol whenFalseLabel)
+        {
+            if (pattern is not BoundNegatedPattern p)
+            {
+                return ImmutableArray.Create(MakeTestsForPattern(1, syntax, input, pattern, null, whenTrueLabel));
+            }
+
+            Tests tests = MakeAndSimplifyTestsAndBindings(input, p.Negated, out ImmutableArray<BoundPatternBinding> bindings);
+            return ImmutableArray.Create(
+                new StateForCase(1, syntax, tests, bindings, null, whenFalseLabel),
+                new StateForCase(2, syntax, Tests.True.Instance, ImmutableArray<BoundPatternBinding>.Empty, null, whenTrueLabel));
+        }
+
         private Tests MakeAndSimplifyTestsAndBindings(
             BoundDagTemp input,
             BoundPattern pattern,

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

@@ -2015,15 +2015,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

@@ -837,8 +837,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);

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.PatternLocalRewriter.cs

@@ -410,7 +410,7 @@ protected BoundDecisionDag ShareTempsAndEvaluateInput(
                 // We share input variables if there is no when clause (because a when clause might mutate them).
                 bool anyWhenClause =
                     decisionDag.TopologicallySortedNodes
-                    .Any(node => node is BoundWhenDecisionDagNode { WhenExpression: { ConstantValue: null } });
+                    .Any(node => node is BoundWhenDecisionDagNode { WhenExpression: { ConstantValue: null } or null });
 
                 var inputDagTemp = BoundDagTemp.ForOriginalInput(loweredInput);
                 if ((loweredInput.Kind == BoundKind.Local || loweredInput.Kind == BoundKind.Parameter)

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IsPatternOperator.cs

@@ -14,7 +14,6 @@ internal sealed partial class LocalRewriter
     {
         public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node)
         {
-            bool negated = node.IsNegated;
             BoundExpression result;
 
             if (canProduceLinearSequence(node.DecisionDag.RootNode, whenTrueLabel: node.WhenTrueLabel, whenFalseLabel: node.WhenFalseLabel))
@@ -28,9 +27,9 @@ public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node
             {
                 // If we can build a linear test sequence with the whenTrue and whenFalse labels swapped, then negate the
                 // result.  This would typically arise when the source contains `e is not pattern`.
-                negated = !negated;
                 var isPatternRewriter = new IsPatternExpressionLinearLocalRewriter(node, this);
                 result = isPatternRewriter.LowerIsPatternAsLinearTestSequence(node, whenTrueLabel: node.WhenFalseLabel, whenFalseLabel: node.WhenTrueLabel);
+                result = this._factory.Not(result);
                 isPatternRewriter.Free();
             }
             else
@@ -41,10 +40,6 @@ public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node
                 isPatternRewriter.Free();
             }
 
-            if (negated)
-            {
-                result = this._factory.Not(result);
-            }
             return result;
 
             // Can the given decision dag node, and its successors, be generated as a sequence of