Distinguish irrefutable and refutable patterns in the is expression

Both refutable and irrefutable is-pattern matches affect definite assignment:
- an irrefutable match leaves pattern variables definitely assigned
- a refutable match is an error
Fixes dotnet#25890

Also, we check to see if a constant input makes the result refutable or irrefutable:
- If a constant cannot match the pattern, we issue a warning
- If a constant always matches a constant pattern, we issue a warning
The latter two are subject to compat council approval.
Fixes dotnet#16099

docs/compilers/CSharp/Compiler Breaking Changes - Milestone 16.md

@@ -15,3 +15,13 @@ Each entry should include a short description of the break, followed by either a
 
  if (o is _) // warning: The name '_' refers to the type '_', not the discard pattern. Use '@_' for the type, or 'var _' to discard.
  ```
+
+3. In an *is-pattern-expression*, a warning is now issued when a constant expression does not match the provided pattern because of its value. Such code was previously accepted but gave no warning. For example
+ ``` c#
+ if (3 is 4) // warning: the given expression never matches the provided pattern.
+ ```
+ We also issue a warning when a constant expression *always* matches a constant pattern in an *is-pattern-expression*. For example
+ ``` c#
+ if (3 is 3) // warning: the given expression always matches the provided constant.
+ ```
+ Other cases of the pattern always matching (e.g. `e is var t`) do not trigger a warning, even when they are known by the compiler to produce an invariant result.

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

@@ -2641,9 +2641,9 @@ private bool IsOperandErrors(CSharpSyntaxNode node, ref BoundExpression operand,
  if (!operand.HasAnyErrors)
  {
  Error(diagnostics, ErrorCode.ERR_LambdaInIsAs, node);
- operand = BadExpression(node, operand).MakeCompilerGenerated();
  }
 
+ operand = BadExpression(node, operand).MakeCompilerGenerated();
  return true;
 
  default:
@@ -2655,6 +2655,7 @@ private bool IsOperandErrors(CSharpSyntaxNode node, ref BoundExpression operand,
  Error(diagnostics, ErrorCode.ERR_BadUnaryOp, node, SyntaxFacts.GetText(SyntaxKind.IsKeyword), operand.Display);
  }
 
+ operand = BadExpression(node, operand).MakeCompilerGenerated();
  return true;
  }
 
@@ -2724,7 +2725,7 @@ private BoundExpression BindIsOperator(BinaryExpressionSyntax node, DiagnosticBa
  {
  isTypeDiagnostics.Free();
  diagnostics.AddRangeAndFree(isPatternDiagnostics);
- return new BoundIsPatternExpression(node, operand, boundConstantPattern, resultType, operandHasErrors);
+ return MakeIsPatternExpression(node, operand, boundConstantPattern, resultType, operandHasErrors, diagnostics);
  }
 
  isPatternDiagnostics.Free();

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

@@ -20,23 +20,53 @@ private BoundExpression BindIsPatternExpression(IsPatternExpressionSyntax node,
  TypeSymbol expressionType = expression.Type;
  if ((object)expressionType == null || expressionType.SpecialType == SpecialType.System_Void)
  {
- expressionType = CreateErrorType();
  if (!hasErrors)
  {
  // value expected
  diagnostics.Add(ErrorCode.ERR_BadPatternExpression, node.Expression.Location, expression.Display);
  hasErrors = true;
  }
+
+ expression = BadExpression(expression.Syntax, expression);
+ }
+
+ BoundPattern pattern = BindPattern(node.Pattern, expression.Type, hasErrors, diagnostics);
+ hasErrors |= pattern.HasErrors;
+ return MakeIsPatternExpression(
+ node, expression, pattern, GetSpecialType(SpecialType.System_Boolean, diagnostics, node),
+ hasErrors, diagnostics);
+ }
+
+ private BoundExpression MakeIsPatternExpression(SyntaxNode node, BoundExpression expression, BoundPattern pattern, TypeSymbol boolType, bool hasErrors, DiagnosticBag diagnostics)
+ {
+ // Note that these labels are for the convenience of the compilation of patterns, and are not actually emitted into the lowered code.
+ LabelSymbol whenTrueLabel = new GeneratedLabelSymbol("isPatternSuccess");
+ LabelSymbol whenFalseLabel = new GeneratedLabelSymbol("isPatternFailure");
+ BoundDecisionDag decisionDag = DecisionDagBuilder.CreateDecisionDagForIsPattern(
+ this.Compilation, pattern.Syntax, expression, pattern, whenTrueLabel: whenTrueLabel, whenFalseLabel: whenFalseLabel, diagnostics);
+ if (!hasErrors && !decisionDag.ReachableLabels.Contains(whenTrueLabel))
+ {
+ diagnostics.Add(ErrorCode.ERR_IsPatternImpossible, node.Location, expression.Type);
+ hasErrors = true;
  }
 
- BoundPattern pattern = BindPattern(node.Pattern, expressionType, hasErrors, diagnostics);
- if (!hasErrors && pattern is BoundDeclarationPattern p && !p.IsVar && expression.ConstantValue == ConstantValue.Null)
+ if (expression.ConstantValue != null)
  {
- diagnostics.Add(ErrorCode.WRN_IsAlwaysFalse, node.Location, p.DeclaredType.Type);
+ decisionDag = decisionDag.SimplifyDecisionDagForConstantInput(expression, this.Conversions, diagnostics);
+ if (!hasErrors)
+ {
+ if (!decisionDag.ReachableLabels.Contains(whenTrueLabel))
+ {
+ diagnostics.Add(ErrorCode.WRN_GivenExpressionNeverMatchesPattern, node.Location);
+ }
+ else if (!decisionDag.ReachableLabels.Contains(whenFalseLabel) && pattern.Kind == BoundKind.ConstantPattern)
+ {
+ diagnostics.Add(ErrorCode.WRN_GivenExpressionAlwaysMatchesConstant, node.Location);
+ }
+ }
  }
 
- return new BoundIsPatternExpression(
- node, expression, pattern, GetSpecialType(SpecialType.System_Boolean, diagnostics, node), hasErrors);
+ return new BoundIsPatternExpression(node, expression, pattern, decisionDag, whenTrueLabel: whenTrueLabel, whenFalseLabel: whenFalseLabel, boolType, hasErrors);
  }
 
  private BoundExpression BindSwitchExpression(SwitchExpressionSyntax node, DiagnosticBag diagnostics)

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

@@ -107,23 +107,22 @@ public static BoundDecisionDag CreateDecisionDagForIsPattern(
  SyntaxNode syntax,
  BoundExpression inputExpression,
  BoundPattern pattern,
- LabelSymbol defaultLabel,
- DiagnosticBag diagnostics,
- out LabelSymbol successLabel)
+ LabelSymbol whenTrueLabel,
+ LabelSymbol whenFalseLabel,
+ DiagnosticBag diagnostics)
  {
- var builder = new DecisionDagBuilder(compilation, defaultLabel, diagnostics);
- return builder.CreateDecisionDagForIsPattern(syntax, inputExpression, pattern, out successLabel);
+ var builder = new DecisionDagBuilder(compilation, defaultLabel: whenFalseLabel, diagnostics);
+ return builder.CreateDecisionDagForIsPattern(syntax, inputExpression, pattern, whenTrueLabel);
  }
 
  private BoundDecisionDag CreateDecisionDagForIsPattern(
  SyntaxNode syntax,
  BoundExpression inputExpression,
  BoundPattern pattern,
- out LabelSymbol successLabel)
+ LabelSymbol whenTrueLabel)
  {
- successLabel = new GeneratedLabelSymbol("success");
  var rootIdentifier = new BoundDagTemp(inputExpression.Syntax, inputExpression.Type, source: null, index: 0);
- return MakeDecisionDag(syntax, ImmutableArray.Create(MakeTestsForPattern(index: 1, pattern.Syntax, rootIdentifier, pattern, whenClause: null, successLabel)));
+ return MakeDecisionDag(syntax, ImmutableArray.Create(MakeTestsForPattern(index: 1, pattern.Syntax, rootIdentifier, pattern, whenClause: null, whenTrueLabel)));
  }
 
  private BoundDecisionDag CreateDecisionDagForSwitchStatement(
@@ -401,10 +400,14 @@ private void MakeTestsAndBindings(
  ArrayBuilder<(BoundExpression, BoundDagTemp)> bindings)
  {
  Debug.Assert(input.Type.IsErrorType() || input.Type == recursive.InputType);
- if (recursive.DeclaredType != null && recursive.DeclaredType.Type != input.Type)
+ if (recursive.DeclaredType != null)
  {
  input = MakeConvertToType(input, recursive.Syntax, recursive.DeclaredType.Type, tests);
  }
+ else
+ {
+ MakeCheckNotNull(input, recursive.Syntax, tests);
+ }
 
  if (!recursive.Deconstruction.IsDefault)
  {

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

@@ -826,6 +826,7 @@
  <Field Name="InnerLocals" Type="ImmutableArray&lt;LocalSymbol&gt;"/>
  <Field Name="InnerLocalFunctions" Type="ImmutableArray&lt;LocalFunctionSymbol&gt;"/>
  <Field Name="SwitchSections" Type="ImmutableArray&lt;BoundPatternSwitchSection&gt;"/>
+ <Field Name="DecisionDag" Type="BoundDecisionDag" Null="disallow" SkipInVisitor="true"/>
  <Field Name="DefaultLabel" Type="BoundPatternSwitchLabel" Null="allow"/>
  <Field Name="BreakLabel" Type="GeneratedLabelSymbol"/>
  </Node>
@@ -862,6 +863,7 @@
  <Node Name="BoundSwitchExpression" Base="BoundExpression">
  <Field Name="Expression" Type="BoundExpression" Null="disallow"/>
  <Field Name="SwitchArms" Type="ImmutableArray&lt;BoundSwitchExpressionArm&gt;"/>
+ <Field Name="DecisionDag" Type="BoundDecisionDag" Null="disallow" SkipInVisitor="true"/>
  <Field Name="DefaultLabel" Type="LabelSymbol" Null="allow"/>
  </Node>
  <Node Name="BoundSwitchExpressionArm" Base="BoundNode">
@@ -1720,6 +1722,9 @@
  <Node Name="BoundIsPatternExpression" Base="BoundExpression">
  <Field Name="Expression" Type="BoundExpression" Null="disallow"/>
  <Field Name="Pattern" Type="BoundPattern" Null="disallow"/>
+ <Field Name="DecisionDag" Type="BoundDecisionDag" Null="disallow" SkipInVisitor="true"/>
+ <Field Name="WhenTrueLabel" Type="LabelSymbol" Null="disallow"/>
+ <Field Name="WhenFalseLabel" Type="LabelSymbol" Null="disallow"/>
  </Node>
 
  <AbstractNode Name="BoundPattern" Base="BoundNode">

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -5372,4 +5372,19 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
  <data name="IDS_FeatureIndexingMovableFixedBuffers" xml:space="preserve">
  <value>indexing movable fixed buffers</value>
  </data>
+ <data name="ERR_IsPatternImpossible" xml:space="preserve">
+ <value>An expression of type '{0}' can never match the provided pattern.</value>
+ </data>
+ <data name="WRN_GivenExpressionNeverMatchesPattern" xml:space="preserve">
+ <value>The given expression never matches the provided pattern.</value>
+ </data>
+ <data name="WRN_GivenExpressionNeverMatchesPattern_Title" xml:space="preserve">
+ <value>The given expression never matches the provided pattern.</value>
+ </data>
+ <data name="WRN_GivenExpressionAlwaysMatchesConstant" xml:space="preserve">
+ <value>The given expression always matches the provided constant.</value>
+ </data>
+ <data name="WRN_GivenExpressionAlwaysMatchesConstant_Title" xml:space="preserve">
+ <value>The given expression always matches the provided constant.</value>
+ </data>
 </root>

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

@@ -1596,6 +1596,9 @@ internal enum ErrorCode
  ERR_ConstantPatternNamedUnderscore = 8412,
  WRN_IsTypeNamedUnderscore = 8413,
  ERR_ExpressionTreeContainsSwitchExpression = 8414,
+ ERR_IsPatternImpossible = 8415,
+ WRN_GivenExpressionNeverMatchesPattern = 8416,
+ WRN_GivenExpressionAlwaysMatchesConstant = 8417,
  #endregion diagnostics introduced for recursive patterns
 
  }

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

@@ -325,6 +325,8 @@ internal static int GetWarningLevel(ErrorCode code)
  case ErrorCode.WRN_TupleBinopLiteralNameMismatch:
  case ErrorCode.WRN_SwitchExpressionNotExhaustive:
  case ErrorCode.WRN_IsTypeNamedUnderscore:
+ case ErrorCode.WRN_GivenExpressionNeverMatchesPattern:
+ case ErrorCode.WRN_GivenExpressionAlwaysMatchesConstant:
  return 1;
  default:
  return 0;

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

@@ -1512,9 +1512,9 @@ protected override LocalState UnreachableState()
 
 #region Visitors
 
- public override void VisitPattern(BoundExpression expression, BoundPattern pattern)
+ public override void VisitPattern(BoundPattern pattern)
  {
- base.VisitPattern(expression, pattern);
+ base.VisitPattern(pattern);
  var whenFail = StateWhenFalse;
  SetState(StateWhenTrue);
  AssignPatternVariables(pattern);
@@ -1560,7 +1560,7 @@ private void AssignPatternVariables(BoundPattern pattern)
  break;
  }
  default:
- break;
+ throw ExceptionUtilities.UnexpectedValue(pattern.Kind);
  }
  }