Skip to content

Commit

Permalink
Infer null state from is object and is { } tests (#40892)
Browse files Browse the repository at this point in the history
  • Loading branch information
cston committed Jan 14, 2020
1 parent 0c4af33 commit 7102821
Show file tree
Hide file tree
Showing 19 changed files with 887 additions and 535 deletions.
15 changes: 11 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Expand Up @@ -579,10 +579,16 @@ private BoundPattern BindRecursivePattern(RecursivePatternSyntax node, TypeSymbo
BindPatternDesignation(
node.Designation, declTypeWithAnnotations, inputValEscape, typeSyntax, diagnostics,
ref hasErrors, out Symbol variableSymbol, out BoundExpression variableAccess);
bool isExplicitNotNullTest =
node.Designation is null &&
boundDeclType is null &&
properties.IsDefaultOrEmpty &&
deconstructMethod is null &&
deconstructionSubpatterns.IsDefault;
return new BoundRecursivePattern(
syntax: node, declaredType: boundDeclType, inputType: inputType, deconstructMethod: deconstructMethod,
syntax: node, declaredType: boundDeclType, deconstructMethod: deconstructMethod,
deconstruction: deconstructionSubpatterns, properties: properties, variable: variableSymbol,
variableAccess: variableAccess, hasErrors: hasErrors);
variableAccess: variableAccess, isExplicitNotNullTest: isExplicitNotNullTest, inputType: inputType, hasErrors: hasErrors);
}

private MethodSymbol BindDeconstructSubpatterns(
Expand Down Expand Up @@ -933,8 +939,9 @@ private BoundPattern BindVarPattern(VarPatternSyntax node, TypeSymbol inputType,
}

return new BoundRecursivePattern(
syntax: node, declaredType: null, inputType: inputType, deconstructMethod: deconstructMethod,
deconstruction: subPatterns.ToImmutableAndFree(), properties: default, variable: null, variableAccess: null, hasErrors: hasErrors);
syntax: node, declaredType: null, deconstructMethod: deconstructMethod,
deconstruction: subPatterns.ToImmutableAndFree(), properties: default, variable: null, variableAccess: null,
isExplicitNotNullTest: false, inputType: inputType, hasErrors: hasErrors);

void addSubpatternsForTuple(ImmutableArray<TypeWithAnnotations> elementTypes)
{
Expand Down
14 changes: 8 additions & 6 deletions src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs
Expand Up @@ -5,7 +5,6 @@
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Text;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;
Expand Down Expand Up @@ -339,7 +338,7 @@ private DecisionDagBuilder(CSharpCompilation compilation, LabelSymbol defaultLab
// Add a null and type test if needed.
if (!declaration.IsVar)
{
input = MakeConvertToType(input, declaration.Syntax, type, tests);
input = MakeConvertToType(input, declaration.Syntax, type, isExplicitTest: false, tests);
}

BoundExpression variableAccess = declaration.VariableAccess;
Expand All @@ -357,12 +356,13 @@ private DecisionDagBuilder(CSharpCompilation compilation, LabelSymbol defaultLab
private static void MakeCheckNotNull(
BoundDagTemp input,
SyntaxNode syntax,
bool isExplicitTest,
ArrayBuilder<BoundDagTest> tests)
{
if (input.Type.CanContainNull())
{
// Add a null test
tests.Add(new BoundDagNonNullTest(syntax, input));
tests.Add(new BoundDagNonNullTest(syntax, isExplicitTest, input));
}
}

Expand All @@ -373,9 +373,10 @@ private DecisionDagBuilder(CSharpCompilation compilation, LabelSymbol defaultLab
BoundDagTemp input,
SyntaxNode syntax,
TypeSymbol type,
bool isExplicitTest,
ArrayBuilder<BoundDagTest> tests)
{
MakeCheckNotNull(input, syntax, tests);
MakeCheckNotNull(input, syntax, isExplicitTest, tests);
if (!input.Type.Equals(type, TypeCompareKind.AllIgnoreOptions))
{
TypeSymbol inputType = input.Type.StrippedType(); // since a null check has already been done
Expand Down Expand Up @@ -412,7 +413,7 @@ private DecisionDagBuilder(CSharpCompilation compilation, LabelSymbol defaultLab
}
else
{
var convertedInput = MakeConvertToType(input, constant.Syntax, constant.Value.Type, tests);
var convertedInput = MakeConvertToType(input, constant.Syntax, constant.Value.Type, isExplicitTest: false, tests);
tests.Add(new BoundDagValueTest(constant.Syntax, constant.ConstantValue, convertedInput));
}
}
Expand All @@ -438,8 +439,9 @@ private static void Assert(bool condition, string message = null)
ArrayBuilder<BoundPatternBinding> bindings)
{
Debug.Assert(input.Type.IsErrorType() || recursive.InputType.IsErrorType() || input.Type.Equals(recursive.InputType, TypeCompareKind.AllIgnoreOptions));

var inputType = recursive.DeclaredType?.Type ?? input.Type.StrippedType();
input = MakeConvertToType(input, recursive.Syntax, inputType, tests);
input = MakeConvertToType(input, recursive.Syntax, inputType, isExplicitTest: recursive.IsExplicitNotNullTest, tests);

if (!recursive.Deconstruction.IsDefault)
{
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
Expand Up @@ -1318,6 +1318,7 @@
</Node>
<Node Name="BoundDagNonNullTest" Base="BoundDagTest">
<!--Check that the input is not null. Used as part of a type test.-->
<Field Name="IsExplicitTest" Type="bool"/>
</Node>
<Node Name="BoundDagExplicitNullTest" Base="BoundDagTest">
<!--Check that the input is null. Used when an explicit null test appears in source.-->
Expand Down Expand Up @@ -1984,6 +1985,7 @@
https://github.com/dotnet/roslyn/issues/13960 . When that is fixed this field can be
removed. -->
<Field Name="VariableAccess" Type="BoundExpression?"/>
<Field Name="IsExplicitNotNullTest" Type="bool"/>
</Node>

<Node Name="BoundITuplePattern" Base="BoundPattern">
Expand Down
Expand Up @@ -3406,7 +3406,7 @@ internal bool EmitNullablePublicOnly

internal bool ShouldEmitNullableAttributes(Symbol symbol)
{
Debug.Assert(symbol is object);
RoslynDebug.Assert(symbol is object);
Debug.Assert(symbol.IsDefinition);

if (symbol.ContainingModule != SourceModule)
Expand Down
Expand Up @@ -696,7 +696,7 @@ private LocalFunctionSymbol GetDeclaredLocalFunction(LocalFunctionStatementSynta

if (_lazyRemappedSymbols.TryGetValue(originalSymbol, out Symbol remappedSymbol))
{
Debug.Assert(remappedSymbol is object);
RoslynDebug.Assert(remappedSymbol is object);
return (T)remappedSymbol;
}

Expand Down
Expand Up @@ -141,8 +141,8 @@ internal void VerifyUpdatedSymbols()
{
var debugText = expr?.Syntax.ToFullString() ?? originalSymbol.ToDisplayString();
Debug.Assert((object)originalSymbol != updatedSymbol, $"Recorded exact same symbol for {debugText}");
Debug.Assert(originalSymbol is object, $"Recorded null original symbol for {debugText}");
Debug.Assert(updatedSymbol is object, $"Recorded null updated symbol for {debugText}");
RoslynDebug.Assert(originalSymbol is object, $"Recorded null original symbol for {debugText}");
RoslynDebug.Assert(updatedSymbol is object, $"Recorded null updated symbol for {debugText}");
Debug.Assert(AreCloseEnough(originalSymbol, updatedSymbol), @$"Symbol for `{debugText}` changed:
Was {originalSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)}
Now {updatedSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)}");
Expand Down Expand Up @@ -197,7 +197,7 @@ internal SnapshotManager ToManagerAndFree()

internal int EnterNewWalker(Symbol symbol)
{
Debug.Assert(symbol is object);
RoslynDebug.Assert(symbol is object);
var previousSlot = _currentWalkerSlot;

// Because we potentially run multiple passes, we
Expand Down
19 changes: 10 additions & 9 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Expand Up @@ -7483,19 +7483,19 @@ public override BoundNode VisitIsOperator(BoundIsOperator node)
Debug.Assert(!this.IsConditionalState);

var operand = node.Operand;
var typeExpr = node.TargetType;

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

var slotBuilder = ArrayBuilder<int>.GetInstance();
GetSlotsToMarkAsNotNullable(operand, slotBuilder);
if (slotBuilder.Count > 0)
Split();
LearnFromNonNullTest(operand, ref StateWhenTrue);
if (typeExpr.Type?.SpecialType == SpecialType.System_Object)
{
Split();
MarkSlotsAsNotNull(slotBuilder, ref StateWhenTrue);
LearnFromNullTest(operand, ref StateWhenFalse);
}
slotBuilder.Free();

VisitTypeExpression(node.TargetType);
VisitTypeExpression(typeExpr);
SetNotNullResult(node);
return result;
}
Expand Down Expand Up @@ -8216,7 +8216,8 @@ private sealed class ExpressionAndSymbolEqualityComparer : IEqualityComparer<(Bo

public bool Equals((BoundNode? expr, Symbol sym) x, (BoundNode? expr, Symbol sym) y)
{
Debug.Assert(x.sym is object && y.sym is object);
RoslynDebug.Assert(x.sym is object);
RoslynDebug.Assert(y.sym is object);

// We specifically use reference equality for the symbols here because the BoundNode should be immutable.
// We should be storing and retrieving the exact same instance of the symbol, not just an "equivalent"
Expand All @@ -8226,7 +8227,7 @@ public bool Equals((BoundNode? expr, Symbol sym) x, (BoundNode? expr, Symbol sym

public int GetHashCode((BoundNode? expr, Symbol sym) obj)
{
Debug.Assert(obj.sym is object);
RoslynDebug.Assert(obj.sym is object);
return Hash.Combine(obj.expr, obj.sym.GetHashCode());
}
}
Expand Down
Expand Up @@ -79,7 +79,6 @@ public override BoundNode VisitITuplePattern(BoundITuplePattern node)
/// </summary>
/// <param name="inputType">Type type of the input expression (before nullable analysis).
/// Used to determine which types can contain null.</param>
/// <returns>true if there is a top-level explicit null check</returns>
private void LearnFromAnyNullPatterns(
int inputSlot,
TypeSymbol inputType,
Expand Down Expand Up @@ -109,6 +108,11 @@ public override BoundNode VisitITuplePattern(BoundITuplePattern node)
break; // nothing to learn
case BoundRecursivePattern rp:
{
if (rp.IsExplicitNotNullTest)
{
LearnFromNullTest(inputSlot, inputType, ref this.State, markDependentSlotsNotNull: false);
}

// for positional part: we only learn from tuples (not Deconstruct)
if (rp.DeconstructMethod is null && !rp.Deconstruction.IsDefault)
{
Expand Down Expand Up @@ -352,6 +356,10 @@ protected override void VisitSwitchSection(BoundSwitchSection node, bool isLastS
if (inputSlot > 0)
{
MarkDependentSlotsNotNull(inputSlot, inputType, ref this.StateWhenFalse);
if (t.IsExplicitTest)
{
LearnFromNullTest(inputSlot, inputType, ref this.StateWhenFalse, markDependentSlotsNotNull: false);
}
learnFromNonNullTest(inputSlot, ref this.StateWhenTrue);
}
gotoNode(p.WhenTrue, this.StateWhenTrue, nodeBelievedReachable);
Expand Down

0 comments on commit 7102821

Please sign in to comment.