Exhaustiveness checking tracks only non-null decision paths (#31093)

Addresses part of #30597
Later, in a separate PR, the null paths will be checked in the nullable walker

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

@@ -36,18 +36,17 @@ internal override BoundExpression BindSwitchExpressionCore(SwitchExpressionSynta
             ImmutableArray<BoundSwitchExpressionArm> switchArms = BindSwitchExpressionArms(node, originalBinder, diagnostics);
             TypeSymbol resultType = InferResultType(switchArms, diagnostics);
             switchArms = AddConversionsToArms(switchArms, resultType, diagnostics);
-            bool hasErrors = CheckSwitchExpressionExhaustive(node, boundInputExpression, switchArms, out BoundDecisionDag decisionDag, out LabelSymbol defaultLabel, diagnostics);
+            bool reportedNonexhaustive = CheckSwitchExpressionExhaustive(node, boundInputExpression, switchArms, out BoundDecisionDag decisionDag, out LabelSymbol defaultLabel, diagnostics);
 
             // When the input is constant, we use that to reshape the decision dag that is returned
             // so that flow analysis will see that some of the cases may be unreachable.
             decisionDag = decisionDag.SimplifyDecisionDagIfConstantInput(boundInputExpression);
-
-            return new BoundSwitchExpression(node, boundInputExpression, switchArms, decisionDag, defaultLabel, resultType, hasErrors);
+            return new BoundSwitchExpression(node, boundInputExpression, switchArms, decisionDag, defaultLabel, reportedNonexhaustive, resultType);
         }
 
         /// <summary>
         /// Build the decision dag, giving an error if some cases are subsumed and an error if the switch expression is not exhaustive.
-        /// Returns true if there were errors.
+        /// Returns true if there was a non-exhaustive warning reported.
         /// </summary>
         /// <param name="node"></param>
         /// <param name="boundInputExpression"></param>
@@ -74,19 +73,45 @@ internal override BoundExpression BindSwitchExpressionCore(SwitchExpressionSynta
                 }
             }
 
-            bool exhaustive = !reachableLabels.Contains(defaultLabel);
-            if (exhaustive)
+            if (!reachableLabels.Contains(defaultLabel))
             {
                 // switch expression is exhaustive; no default label needed.
                 defaultLabel = null;
+                return false;
             }
-            else
+
+            // We only report exhaustive warnings when the default label is reachable through some series of
+            // tests that do not include a test in which the value is know to be null.  Handling paths with
+            // nulls is the job of the nullable walker.
+            foreach (var n in TopologicalSort.IterativeSort<BoundDecisionDagNode>(new[] { decisionDag.RootNode }, nonNullSuccessors))
             {
-                // warning: switch expression is not exhaustive
-                diagnostics.Add(ErrorCode.WRN_SwitchExpressionNotExhaustive, node.SwitchKeyword.GetLocation());
+                if (n is BoundLeafDecisionDagNode leaf && leaf.Label == defaultLabel)
+                {
+                    diagnostics.Add(ErrorCode.WRN_SwitchExpressionNotExhaustive, node.SwitchKeyword.GetLocation());
+                    return true;
+                }
             }
 
-            return !exhaustive;
+            return false;
+
+            ImmutableArray<BoundDecisionDagNode> nonNullSuccessors(BoundDecisionDagNode n)
+            {
+                switch (n)
+                {
+                    case BoundTestDecisionDagNode p:
+                        switch (p.Test)
+                        {
+                            case BoundDagNonNullTest t: // checks that the input is not null
+                                return ImmutableArray.Create(p.WhenTrue);
+                            case BoundDagNullTest t: // checks that the input is null
+                                return ImmutableArray.Create(p.WhenFalse);
+                            default:
+                                return BoundDecisionDag.Successors(n);
+                        }
+                    default:
+                        return BoundDecisionDag.Successors(n);
+                }
+            }
         }
 
         /// <summary>

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

@@ -16,7 +16,7 @@ internal partial class BoundDecisionDag
         private ImmutableHashSet<LabelSymbol> _reachableLabels;
         private ImmutableArray<BoundDecisionDagNode> _topologicallySortedNodes;
 
-        private static ImmutableArray<BoundDecisionDagNode> Successors(BoundDecisionDagNode node)
+        internal static ImmutableArray<BoundDecisionDagNode> Successors(BoundDecisionDagNode node)
         {
             switch (node)
             {

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

@@ -1149,6 +1149,7 @@
     <Field Name="SwitchArms" Type="ImmutableArray&lt;BoundSwitchExpressionArm&gt;"/>
     <Field Name="DecisionDag" Type="BoundDecisionDag" Null="disallow" SkipInVisitor="true"/>
     <Field Name="DefaultLabel" Type="LabelSymbol" Null="allow"/>
+    <Field Name="ReportedNotExhaustive" Type="bool"/>
   </Node>
   <Node Name="BoundSwitchExpressionArm" Base="BoundNode">
     <Field Name="Locals" Type="ImmutableArray&lt;LocalSymbol&gt;"/>

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -1662,7 +1662,5 @@ internal enum ErrorCode
         #endregion diagnostics introduced for C# 8.0
 
         // Note: you will need to re-generate compiler code after adding warnings (build\scripts\generate-compiler-code.cmd)
-
-        // Note: you will need to re-generate compiler code after adding warnings (build\scripts\generate-compiler-code.cmd)
     }
 }

src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs

@@ -4102,7 +4102,7 @@ public BoundConditionalGoto Update(BoundExpression condition, bool jumpIfTrue, L
 
     internal sealed partial class BoundSwitchExpression : BoundExpression
     {
-        public BoundSwitchExpression(SyntaxNode syntax, BoundExpression expression, ImmutableArray<BoundSwitchExpressionArm> switchArms, BoundDecisionDag decisionDag, LabelSymbol defaultLabel, TypeSymbol type, bool hasErrors = false)
+        public BoundSwitchExpression(SyntaxNode syntax, BoundExpression expression, ImmutableArray<BoundSwitchExpressionArm> switchArms, BoundDecisionDag decisionDag, LabelSymbol defaultLabel, bool reportedNotExhaustive, TypeSymbol type, bool hasErrors = false)
             : base(BoundKind.SwitchExpression, syntax, type, hasErrors || expression.HasErrors() || switchArms.HasErrors() || decisionDag.HasErrors())
         {
 
@@ -4114,6 +4114,7 @@ public BoundSwitchExpression(SyntaxNode syntax, BoundExpression expression, Immu
             this.SwitchArms = switchArms;
             this.DecisionDag = decisionDag;
             this.DefaultLabel = defaultLabel;
+            this.ReportedNotExhaustive = reportedNotExhaustive;
         }
 
 
@@ -4125,16 +4126,18 @@ public BoundSwitchExpression(SyntaxNode syntax, BoundExpression expression, Immu
 
         public LabelSymbol DefaultLabel { get; }
 
+        public bool ReportedNotExhaustive { get; }
+
         public override BoundNode Accept(BoundTreeVisitor visitor)
         {
             return visitor.VisitSwitchExpression(this);
         }
 
-        public BoundSwitchExpression Update(BoundExpression expression, ImmutableArray<BoundSwitchExpressionArm> switchArms, BoundDecisionDag decisionDag, LabelSymbol defaultLabel, TypeSymbol type)
+        public BoundSwitchExpression Update(BoundExpression expression, ImmutableArray<BoundSwitchExpressionArm> switchArms, BoundDecisionDag decisionDag, LabelSymbol defaultLabel, bool reportedNotExhaustive, TypeSymbol type)
         {
-            if (expression != this.Expression || switchArms != this.SwitchArms || decisionDag != this.DecisionDag || defaultLabel != this.DefaultLabel || type != this.Type)
+            if (expression != this.Expression || switchArms != this.SwitchArms || decisionDag != this.DecisionDag || defaultLabel != this.DefaultLabel || reportedNotExhaustive != this.ReportedNotExhaustive || type != this.Type)
             {
-                var result = new BoundSwitchExpression(this.Syntax, expression, switchArms, decisionDag, defaultLabel, type, this.HasErrors);
+                var result = new BoundSwitchExpression(this.Syntax, expression, switchArms, decisionDag, defaultLabel, reportedNotExhaustive, type, this.HasErrors);
                 result.WasCompilerGenerated = this.WasCompilerGenerated;
                 return result;
             }
@@ -10684,7 +10687,7 @@ public override BoundNode VisitSwitchExpression(BoundSwitchExpression node)
             ImmutableArray<BoundSwitchExpressionArm> switchArms = (ImmutableArray<BoundSwitchExpressionArm>)this.VisitList(node.SwitchArms);
             BoundDecisionDag decisionDag = node.DecisionDag;
             TypeSymbol type = this.VisitType(node.Type);
-            return node.Update(expression, switchArms, decisionDag, node.DefaultLabel, type);
+            return node.Update(expression, switchArms, decisionDag, node.DefaultLabel, node.ReportedNotExhaustive, type);
         }
         public override BoundNode VisitSwitchExpressionArm(BoundSwitchExpressionArm node)
         {
@@ -12186,6 +12189,7 @@ public override TreeDumperNode VisitSwitchExpression(BoundSwitchExpression node,
                 new TreeDumperNode("switchArms", null, from x in node.SwitchArms select Visit(x, null)),
                 new TreeDumperNode("decisionDag", null, new TreeDumperNode[] { Visit(node.DecisionDag, null) }),
                 new TreeDumperNode("defaultLabel", node.DefaultLabel, null),
+                new TreeDumperNode("reportedNotExhaustive", node.ReportedNotExhaustive, null),
                 new TreeDumperNode("type", node.Type, null)
             }
             );

src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests4.cs

@@ -1384,5 +1384,108 @@ class _
                 );
             CompileAndVerify(compilation, expectedOutput: "8");
         }
+
+        [Fact]
+        public void IgnoreNullInExhaustiveness_01()
+        {
+            var source =
+@"class Program
+{
+    static void Main() {}
+    static int M1(bool? b1, bool? b2)
+    {
+        return (b1, b2) switch {
+            (false, false) => 1,
+            (false, true) => 2,
+            // (true, false) => 3,
+            (true, true) => 4
+            };
+    }
+}
+";
+            var compilation = CreatePatternCompilation(source);
+            compilation.VerifyDiagnostics(
+                // (6,25): warning CS8509: The switch expression does not handle all possible inputs (it is not exhaustive).
+                //         return (b1, b2) switch {
+                Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithLocation(6, 25)
+                );
+        }
+
+        [Fact]
+        public void IgnoreNullInExhaustiveness_02()
+        {
+            var source =
+@"class Program
+{
+    static void Main() {}
+    static int M1(bool? b1, bool? b2)
+    {
+        return (b1, b2) switch {
+            (false, false) => 1,
+            (false, true) => 2,
+            (true, false) => 3,
+            (true, true) => 4
+            };
+    }
+}
+";
+            var compilation = CreatePatternCompilation(source);
+            compilation.VerifyDiagnostics(
+                );
+        }
+
+        [Fact]
+        public void IgnoreNullInExhaustiveness_03()
+        {
+            var source =
+@"class Program
+{
+    static void Main() {}
+    static int M1(bool? b1, bool? b2)
+    {
+        (bool? b1, bool? b2)? cond = (b1, b2);
+        return cond switch {
+            (false, false) => 1,
+            (false, true) => 2,
+            (true, false) => 3,
+            (true, true) => 4,
+            (null, true) => 5
+            };
+    }
+}
+";
+            var compilation = CreatePatternCompilation(source);
+            compilation.VerifyDiagnostics(
+                );
+        }
+
+        [Fact]
+        public void IgnoreNullInExhaustiveness_04()
+        {
+            var source =
+@"class Program
+{
+    static void Main() {}
+    static int M1(bool? b1, bool? b2)
+    {
+        (bool? b1, bool? b2)? cond = (b1, b2);
+        return cond switch {
+            (false, false) => 1,
+            (false, true) => 2,
+            (true, false) => 3,
+            (true, true) => 4,
+            _ => 5,
+            (null, true) => 6
+            };
+    }
+}
+";
+            var compilation = CreatePatternCompilation(source);
+            compilation.VerifyDiagnostics(
+                // (13,13): error CS8510: The pattern has already been handled by a previous arm of the switch expression.
+                //             (null, true) => 6
+                Diagnostic(ErrorCode.ERR_SwitchArmSubsumed, "(null, true)").WithLocation(13, 13)
+                );
+        }
     }
 }