Skip to content

Commit

Permalink
Handle 'is' operators and patterns in definite assignment (#52616)
Browse files Browse the repository at this point in the history
  • Loading branch information
RikkiGibson authored Apr 20, 2021
1 parent 66fcef4 commit 667fb28
Show file tree
Hide file tree
Showing 4 changed files with 762 additions and 5 deletions.
126 changes: 124 additions & 2 deletions src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -962,11 +962,42 @@ public override BoundNode VisitPassByCopy(BoundPassByCopy node)
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);

if (VisitPossibleConditionalAccess(node.Expression, out var stateWhenNotNull))
{
Debug.Assert(!IsConditionalState);
switch (isTopLevelNonNullTest(pattern))
{
case true:
SetConditionalState(stateWhenNotNull, State);
break;
case false:
SetConditionalState(State, stateWhenNotNull);
break;
}
}
else if (IsConditionalState)
{
// Patterns which only match a single boolean value should propagate conditional state
// for example, `(a != null && a.M(out x)) is true` should have the same conditional state as `(a != null && a.M(out x))`.
if (isBoolTest(pattern) is bool value)
{
if (!value)
{
SetConditionalState(StateWhenFalse, StateWhenTrue);
}
}
else
{
// Patterns which match more than a single boolean value cannot propagate conditional state
// for example, `(a != null && a.M(out x)) is bool b` should not have conditional state
Unsplit();
}
}

VisitPattern(pattern);
var reachableLabels = node.DecisionDag.ReachableLabels;
if (!reachableLabels.Contains(node.WhenTrueLabel))
Expand All @@ -986,6 +1017,88 @@ public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node
}

return node;

// Returns `true` if the pattern only matches if the top-level input is not null.
// Returns `false` if the pattern only matches if the top-level input is null.
// Otherwise, returns `null`.
static bool? isTopLevelNonNullTest(BoundPattern pattern)
{
switch (pattern)
{
case BoundTypePattern:
case BoundRecursivePattern:
case BoundITuplePattern:
case BoundRelationalPattern:
case BoundDeclarationPattern { IsVar: false }:
case BoundConstantPattern { ConstantValue: { IsNull: false } }:
return true;
case BoundConstantPattern { ConstantValue: { IsNull: true } }:
return false;
case BoundNegatedPattern negated:
return !isTopLevelNonNullTest(negated.Negated);
case BoundBinaryPattern binary:
if (binary.Disjunction)
{
// `a?.b(out x) is null or C`
// both subpatterns must have the same null test for the test to propagate out
var leftNullTest = isTopLevelNonNullTest(binary.Left);
return leftNullTest is null ? null :
leftNullTest != isTopLevelNonNullTest(binary.Right) ? null :
leftNullTest;
}

// `a?.b(out x) is not null and var c`
// if any pattern performs a test, we know that test applies at the top level
// note that if the tests are different, e.g. `null and not null`,
// the pattern is a contradiction, so we expect an error diagnostic
return isTopLevelNonNullTest(binary.Left) ?? isTopLevelNonNullTest(binary.Right);
case BoundDeclarationPattern { IsVar: true }:
case BoundDiscardPattern:
return null;
default:
throw ExceptionUtilities.UnexpectedValue(pattern.Kind);
}
}

// Returns `true` if the pattern only matches a `true` input.
// Returns `false` if the pattern only matches a `false` input.
// Otherwise, returns `null`.
static bool? isBoolTest(BoundPattern pattern)
{
switch (pattern)
{
case BoundConstantPattern { ConstantValue: { IsBoolean: true, BooleanValue: var boolValue } }:
return boolValue;
case BoundNegatedPattern negated:
return !isBoolTest(negated.Negated);
case BoundBinaryPattern binary:
if (binary.Disjunction)
{
// `(a != null && a.b(out x)) is true or true` matches `true`
// `(a != null && a.b(out x)) is true or false` matches any boolean
// both subpatterns must have the same bool test for the test to propagate out
var leftNullTest = isBoolTest(binary.Left);
return leftNullTest is null ? null :
leftNullTest != isBoolTest(binary.Right) ? null :
leftNullTest;
}

// `(a != null && a.b(out x)) is true and true` matches `true`
// `(a != null && a.b(out x)) is true and var x` matches `true`
// `(a != null && a.b(out x)) is true and false` never matches and is a compile error
return isBoolTest(binary.Left) ?? isBoolTest(binary.Right);
case BoundConstantPattern { ConstantValue: { IsBoolean: false } }:
case BoundDiscardPattern:
case BoundTypePattern:
case BoundRecursivePattern:
case BoundITuplePattern:
case BoundRelationalPattern:
case BoundDeclarationPattern:
return null;
default:
throw ExceptionUtilities.UnexpectedValue(pattern.Kind);
}
}
}

public virtual void VisitPattern(BoundPattern pattern)
Expand Down Expand Up @@ -2510,7 +2623,16 @@ public override BoundNode VisitAsOperator(BoundAsOperator node)

public override BoundNode VisitIsOperator(BoundIsOperator node)
{
VisitRvalue(node.Operand);
if (VisitPossibleConditionalAccess(node.Operand, out var stateWhenNotNull))
{
Debug.Assert(!IsConditionalState);
SetConditionalState(stateWhenNotNull, State);
}
else
{
// `(a && b.M(out x)) is bool` should discard conditional state from LHS
Unsplit();
}
return null;
}

Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9105,7 +9105,7 @@ private TypeWithState InferResultNullabilityOfBinaryLogicalOperator(BoundExpress
var operand = node.Operand;
var typeExpr = node.TargetType;

var result = base.VisitIsOperator(node);
VisitRvalue(operand);
Debug.Assert(node.Type.SpecialType == SpecialType.System_Boolean);

Split();
Expand All @@ -9117,7 +9117,7 @@ private TypeWithState InferResultNullabilityOfBinaryLogicalOperator(BoundExpress

VisitTypeExpression(typeExpr);
SetNotNullResult(node);
return result;
return null;
}

public override BoundNode? VisitAsOperator(BoundAsOperator node)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public override BoundNode VisitRecursivePattern(BoundRecursivePattern node)

public override BoundNode VisitConstantPattern(BoundConstantPattern node)
{
Visit(node.Value);
VisitRvalue(node.Value);
return null;
}

Expand Down
Loading

0 comments on commit 667fb28

Please sign in to comment.