Skip to content

Commit

Permalink
Fix regression with var pattern nullability
Browse files Browse the repository at this point in the history
  • Loading branch information
jcouv committed May 26, 2021
1 parent d513ddf commit ccef3a7
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ protected override LocalState VisitSwitchStatementDispatch(BoundSwitchStatement
// visit switch header
Visit(node.Expression);
var expressionState = ResultType;
var initialState = PossiblyConditionalState.Create(this);

DeclareLocals(node.InnerLocals);
foreach (var section in node.SwitchSections)
Expand All @@ -217,7 +216,7 @@ protected override LocalState VisitSwitchStatementDispatch(BoundSwitchStatement
DeclareLocals(section.Locals);
}

var labelStateMap = LearnFromDecisionDag(node.Syntax, node.DecisionDag, node.Expression, expressionState, ref initialState);
var labelStateMap = LearnFromDecisionDag(node.Syntax, node.DecisionDag, node.Expression, expressionState);
foreach (var section in node.SwitchSections)
{
foreach (var label in section.SwitchLabels)
Expand Down Expand Up @@ -284,13 +283,11 @@ public PossiblyConditionalState Clone()
}
}

private PooledDictionary<LabelSymbol, (LocalState state, bool believedReachable)>
LearnFromDecisionDag(
private PooledDictionary<LabelSymbol, (LocalState state, bool believedReachable)> LearnFromDecisionDag(
SyntaxNode node,
BoundDecisionDag decisionDag,
BoundExpression expression,
TypeWithState expressionType,
ref PossiblyConditionalState initialState)
TypeWithState expressionType)
{
// We reuse the slot at the beginning of a switch (or is-pattern expression), pretending that we are
// not copying the input to evaluate the patterns. In this way we infer non-nullability of the original
Expand Down Expand Up @@ -319,7 +316,7 @@ public PossiblyConditionalState Clone()
tempMap.Add(rootTemp, (originalInputSlot, expressionType.Type));

var nodeStateMap = PooledDictionary<BoundDecisionDagNode, (PossiblyConditionalState state, bool believedReachable)>.GetInstance();
nodeStateMap.Add(decisionDag.RootNode, (state: initialState.Clone(), believedReachable: true));
nodeStateMap.Add(decisionDag.RootNode, (state: PossiblyConditionalState.Create(this), believedReachable: true));

var labelStateMap = PooledDictionary<LabelSymbol, (LocalState state, bool believedReachable)>.GetInstance();

Expand Down Expand Up @@ -735,8 +732,7 @@ private void VisitSwitchExpressionCore(BoundSwitchExpression node, bool inferTyp

Visit(node.Expression);
var expressionState = ResultType;
var state = PossiblyConditionalState.Create(this);
var labelStateMap = LearnFromDecisionDag(node.Syntax, node.DecisionDag, node.Expression, expressionState, ref state);
var labelStateMap = LearnFromDecisionDag(node.Syntax, node.DecisionDag, node.Expression, expressionState);
var endState = UnreachableState();

if (!node.ReportedNotExhaustive && node.DefaultLabel != null &&
Expand Down Expand Up @@ -846,8 +842,7 @@ public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node
VisitPatternForRewriting(node.Pattern);
Visit(node.Expression);
var expressionState = ResultType;
var state = PossiblyConditionalState.Create(this);
var labelStateMap = LearnFromDecisionDag(node.Syntax, node.DecisionDag, node.Expression, expressionState, ref state);
var labelStateMap = LearnFromDecisionDag(node.Syntax, node.DecisionDag, node.Expression, expressionState);
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();
labelStateMap.Free();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146719,5 +146719,71 @@ void M(I3 i)
AssertEx.Equal(new string[] { "I<object?>", "I2<object!>", "I<object!>" },
i3.AllInterfaces().ToTestDisplayStrings(TypeWithAnnotations.TestDisplayFormat));
}

[Fact]
public void VarPatternDeclaration_TopLevel()
{
var src = @"
#nullable enable
public class C
{
public void M(string? x)
{
if (Identity(x) is var y)
{
y.ToString(); // 1
}

if (Identity(x) is not null and var z)
{
z.ToString();
}
}

public T Identity<T>(T t) => t;
}
";
var comp = CreateCompilation(src);
comp.VerifyDiagnostics(
// (9,13): warning CS8602: Dereference of a possibly null reference.
// y.ToString(); // 1
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "y").WithLocation(9, 13)
);
}

[Fact]
public void VarPatternDeclaration_Nested()
{
var src = @"
#nullable enable
public class Container<T>
{
public T Item { get; set; } = default!;
}
public class C
{
public void M(Container<string?> x)
{
if (Identity(x) is { Item: var y })
{
y.ToString(); // 1
}

if (Identity(x) is { Item: not null and var z })
{
z.ToString();
}
}

public T Identity<T>(T t) => t;
}
";
var comp = CreateCompilation(src);
comp.VerifyDiagnostics(
// (13,13): warning CS8602: Dereference of a possibly null reference.
// y.ToString(); // 1
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "y").WithLocation(13, 13)
);
}
}
}

0 comments on commit ccef3a7

Please sign in to comment.