Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

is operator informs nullability analysis #28686

Merged
merged 7 commits into from
Jul 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion docs/features/nullable-reference-types.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,17 @@ Flow analysis is used to infer the nullability of variables within executable co

### Warnings
_Describe set of warnings. Differentiate W warnings._
If the analysis determines that a null check always (or never) passes, a hidden diagnostic is produced. For example: `"string" is null`.

### Null tests
_Describe the set of tests that affect flow state._
A number of null checks affect the flow state when tested for:
- comparisons to `null`: `x == null` and `x != null`
- `is` operator: `x is null`, `x is K` (where `K` is a constant), `x is string`, `x is string s`

Invocation of methods annotated with the following attributes will also affect flow analysis:
- `[NotNullWhenTrue]` (e.g. `TryGetValue`) and `[NotNullWhenFalse]` (e.g. `string.IsNullOrEmpty`)
- `[EnsuresNotNull]` (e.g. `ThrowIfNull`)
- `[AssertsTrue]` (e.g. `Debug.Assert`) and `[AssertsFalse]`

## `default`
`default(T)` is `T?` if `T` is a reference type.
Expand Down
140 changes: 90 additions & 50 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -766,69 +766,83 @@ private void EnterParameter(ParameterSymbol parameter, TypeSymbolWithAnnotations

public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node)
{
// PROTOTYPE(NullableReferenceTypes): Move these asserts to base class.
Debug.Assert(!IsConditionalState);

// Create slot when the state is unconditional since EnsureCapacity should be
// called on all fields and that is simpler if state is limited to this.State.
int slot = -1;
if (this.State.Reachable)
{
var pattern = node.Pattern;
// PROTOTYPE(NullableReferenceTypes): Handle patterns that ensure x is not null:
// x is T y // where T is not inferred via var
// x is K // where K is a constant other than null
if (pattern.Kind == BoundKind.ConstantPattern && ((BoundConstantPattern)pattern).ConstantValue?.IsNull == true)
{
slot = MakeSlot(node.Expression);
if (slot > 0)
{
Normalize(ref this.State);
}
}
}
VisitRvalue(node.Expression);
var expressionResultType = this._resultType;

var result = base.VisitIsPatternExpression(node);

Debug.Assert(IsConditionalState);
if (slot > 0)
{
this.StateWhenTrue[slot] = false;
this.StateWhenFalse[slot] = true;
}
VisitPattern(node.Expression, expressionResultType, node.Pattern);

SetResult(node);
return result;
return node;
}

public override void VisitPattern(BoundExpression expression, BoundPattern pattern)
{
base.VisitPattern(expression, pattern);
var whenFail = StateWhenFalse;
SetState(StateWhenTrue);
AssignPatternVariables(pattern);
SetConditionalState(this.State, whenFail);
SetUnknownResultNullability();
}

private void AssignPatternVariables(BoundPattern pattern)
/// <summary>
/// Examples:
/// `x is Point p`
/// `switch (x) ... case Point p:` // PROTOTYPE(NullableReferenceTypes): not yet handled
///
/// If the expression is trackable, we'll return with different null-states for that expression in the two conditional states.
/// If the pattern is a `var` pattern, we'll also have re-inferred the `var` type with nullability and
/// updated the state for that declared local.
/// </summary>
private void VisitPattern(BoundExpression expression, TypeSymbolWithAnnotations expressionResultType, BoundPattern pattern)
{
bool? isNull = null; // the pattern tells us the expression (1) is null, (2) isn't null, or (3) we don't know.
switch (pattern.Kind)
{
case BoundKind.DeclarationPattern:
// PROTOTYPE(NullableReferenceTypes): Handle.
break;
case BoundKind.WildcardPattern:
break;
case BoundKind.ConstantPattern:
// If the constant is null, the pattern tells us the expression is null.
// If the constant is not null, the pattern tells us the expression is not null.
// If there is no constant, we don't know.
isNull = ((BoundConstantPattern)pattern).ConstantValue?.IsNull;
break;
case BoundKind.DeclarationPattern:
var declarationPattern = (BoundDeclarationPattern)pattern;
if (declarationPattern.IsVar)
{
var pat = (BoundConstantPattern)pattern;
this.VisitRvalue(pat.Value);
break;
// The result type and state of the expression carry into the variable declared by var pattern
Symbol variable = declarationPattern.Variable;
// No variable declared for discard (`i is var _`)
if ((object)variable != null)
{
_variableTypes[variable] = expressionResultType;
TrackNullableStateForAssignment(expression, expressionResultType, GetOrCreateSlot(variable), expressionResultType);
}
}
else
{
isNull = false; // the pattern tells us the expression is not null
}
default:
break;
}

Debug.Assert(!IsConditionalState);
int slot = -1;
if (isNull != null)
{
// Create slot when the state is unconditional since EnsureCapacity should be
// called on all fields and that is simpler if state is limited to this.State.
slot = MakeSlot(expression);
if (slot > 0)
{
Normalize(ref this.State);
}
}

base.VisitPattern(expression, pattern);
Debug.Assert(IsConditionalState);

// PROTOTYPE(NullableReferenceTypes): We should only report such
// diagnostics for locals that are set or checked explicitly within this method.
if (expressionResultType?.IsNullable == false && isNull == true)
{
ReportStaticNullCheckingDiagnostics(ErrorCode.HDN_NullCheckIsProbablyAlwaysFalse, pattern.Syntax);
}

if (slot > 0 && isNull != null)
{
this.StateWhenTrue[slot] = !isNull;
this.StateWhenFalse[slot] = isNull;
}
}

protected override BoundNode VisitReturnStatementNoAdjust(BoundReturnStatement node)
Expand Down Expand Up @@ -3663,8 +3677,29 @@ public override BoundNode VisitDefaultExpression(BoundDefaultExpression node)

public override BoundNode VisitIsOperator(BoundIsOperator node)
{
Debug.Assert(!this.IsConditionalState);

int slot = -1;
var operand = node.Operand;
if (operand.Type?.IsValueType == false)
{
slot = MakeSlot(operand);
if (slot > 0)
{
Normalize(ref this.State);
}
}

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

if (slot > 0)
{
Split();
this.StateWhenTrue[slot] = true;
this.StateWhenFalse[slot] = false;
}

SetResult(node);
return result;
}
Expand Down Expand Up @@ -4185,6 +4220,11 @@ internal void EnsureCapacity(int capacity)
_notNull.EnsureCapacity(capacity);
}

/// <summary>
/// Use `true` value for not-null.
/// Use `false` value for maybe-null.
/// Use `null` value for unknown.
/// </summary>
internal bool? this[int slot]
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -950,9 +950,10 @@ public override BoundNode VisitPassByCopy(BoundPassByCopy node)

public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node)
{
Debug.Assert(!IsConditionalState);
VisitRvalue(node.Expression);
VisitPattern(node.Expression, node.Pattern);
return null;
return node;
}

public virtual void VisitPattern(BoundExpression expression, BoundPattern pattern)
Expand Down
Loading