Skip to content

Commit

Permalink
Lifted comparison operator implies operands non-null when true
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv committed May 19, 2022
1 parent 67e035e commit 415427a
Show file tree
Hide file tree
Showing 2 changed files with 179 additions and 13 deletions.
36 changes: 23 additions & 13 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4463,11 +4463,21 @@ void visitOperandConversion(
}

Debug.Assert(!IsConditionalState);
if (binary.OperatorKind.IsLifted()
&& binary.OperatorKind.Operator() is BinaryOperatorKind.GreaterThan or BinaryOperatorKind.GreaterThanOrEqual or BinaryOperatorKind.LessThan or BinaryOperatorKind.LessThanOrEqual)
{
Debug.Assert(binary.Type.SpecialType == SpecialType.System_Boolean);
SplitAndLearnFromNonNullTest(binary.Left, whenTrue: true);
SplitAndLearnFromNonNullTest(binary.Right, whenTrue: true);
}

// For nested binary operators, this can be the only time they're visited due to explicit stack used in AbstractFlowPass.VisitBinaryOperator,
// so we need to set the flow-analyzed type here.
var inferredResult = InferResultNullability(binary.OperatorKind, method, binary.Type, leftType, rightType);
SetResult(binary, inferredResult, inferredResult.ToTypeWithAnnotations(compilation));

return;

TypeSymbol? getTypeIfContainingType(TypeSymbol baseType, TypeSymbol? derivedType, BoundExpression operand)
{
if (derivedType is null || IsTargetTypedExpression(operand))
Expand Down Expand Up @@ -4518,7 +4528,7 @@ private void AfterLeftChildHasBeenVisited(
{
// Set all nested conditional slots. For example in a?.b?.c we'll set a, b, and c.
bool nonNullCase = op != BinaryOperatorKind.Equal; // true represents WhenTrue
splitAndLearnFromNonNullTest(operandComparedToNull, whenTrue: nonNullCase);
SplitAndLearnFromNonNullTest(operandComparedToNull, whenTrue: nonNullCase);

// `x == null` and `x != null` are pure null tests so update the null-state in the alternative branch too
LearnFromNullTest(operandComparedToNull, ref nonNullCase ? ref StateWhenFalse : ref StateWhenTrue);
Expand Down Expand Up @@ -4547,27 +4557,27 @@ private void AfterLeftChildHasBeenVisited(
case BinaryOperatorKind.GreaterThanOrEqual:
case BinaryOperatorKind.LessThanOrEqual:
operandComparedToNonNull = SkipReferenceConversions(operandComparedToNonNull);
splitAndLearnFromNonNullTest(operandComparedToNonNull, whenTrue: true);
SplitAndLearnFromNonNullTest(operandComparedToNonNull, whenTrue: true);
return;
case BinaryOperatorKind.NotEqual:
operandComparedToNonNull = SkipReferenceConversions(operandComparedToNonNull);
splitAndLearnFromNonNullTest(operandComparedToNonNull, whenTrue: false);
SplitAndLearnFromNonNullTest(operandComparedToNonNull, whenTrue: false);
return;
};
}
}

void splitAndLearnFromNonNullTest(BoundExpression operandComparedToNonNull, bool whenTrue)
private void SplitAndLearnFromNonNullTest(BoundExpression operandComparedToNonNull, bool whenTrue)
{
var slotBuilder = ArrayBuilder<int>.GetInstance();
GetSlotsToMarkAsNotNullable(operandComparedToNonNull, slotBuilder);
if (slotBuilder.Count != 0)
{
var slotBuilder = ArrayBuilder<int>.GetInstance();
GetSlotsToMarkAsNotNullable(operandComparedToNonNull, slotBuilder);
if (slotBuilder.Count != 0)
{
Split();
ref LocalState stateToUpdate = ref whenTrue ? ref this.StateWhenTrue : ref this.StateWhenFalse;
MarkSlotsAsNotNull(slotBuilder, ref stateToUpdate);
}
slotBuilder.Free();
Split();
ref LocalState stateToUpdate = ref whenTrue ? ref this.StateWhenTrue : ref this.StateWhenFalse;
MarkSlotsAsNotNull(slotBuilder, ref stateToUpdate);
}
slotBuilder.Free();
}

protected override bool VisitInterpolatedStringHandlerParts(BoundInterpolatedStringBase node, bool usesBoolReturns, bool firstPartIsConditional, ref LocalState shortCircuitState)
Expand Down
156 changes: 156 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156109,5 +156109,161 @@ void F(object? o)
Diagnostic(ErrorCode.ERR_ImplicitlyTypedArrayNoBestType, "stackalloc[] { x => x }").WithLocation(9, 18)
);
}

[Fact, WorkItem(61361, "https://github.com/dotnet/roslyn/issues/61361")]
public void NullableBoolInVariousTests()
{
var comp = CreateCompilation(@"
#nullable enable
class C
{
public static void M(bool? x)
{
if (x) // 1
{
x.Value.ToString(); // 2
}

_ = x ? x.Value.ToString() : string.Empty; // 3, 4

if (x == true)
{
x.Value.ToString();
}

_ = (x == true) ? x.Value.ToString() : string.Empty;

_ = x switch
{
true => x.Value.ToString(),
_ => string.Empty
};
}
}
");

comp.VerifyDiagnostics(
// (7,13): error CS0266: Cannot implicitly convert type 'bool?' to 'bool'. An explicit conversion exists (are you missing a cast?)
// if (x) // 1
Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "x").WithArguments("bool?", "bool").WithLocation(7, 13),
// (9,13): warning CS8629: Nullable value type may be null.
// x.Value.ToString(); // 2
Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "x").WithLocation(9, 13),
// (12,13): error CS0266: Cannot implicitly convert type 'bool?' to 'bool'. An explicit conversion exists (are you missing a cast?)
// _ = x ? x.Value.ToString() : string.Empty; // 3, 4
Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "x").WithArguments("bool?", "bool").WithLocation(12, 13),
// (12,17): warning CS8629: Nullable value type may be null.
// _ = x ? x.Value.ToString() : string.Empty; // 3, 4
Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "x").WithLocation(12, 17)
);
}

[Theory, WorkItem(61361, "https://github.com/dotnet/roslyn/issues/61361")]
[InlineData("<")]
[InlineData("<=")]
[InlineData(">")]
[InlineData(">=")]
public void LiftedRelationalOperation(string op)
{
var comp = CreateCompilation($$"""
#nullable enable
class C
{
public static void Test(int? first, int? second)
{
if (first {{op}} second)
{
first.Value.ToString();
second.Value.ToString();
}

_ = (first {{op}} second) ? (first.Value.ToString() + second.Value.ToString()) : string.Empty;

_ = (first {{op}} second) switch
{
true => first.Value.ToString() + second.Value.ToString(),
_ => string.Empty
};
}
}
""");
comp.VerifyDiagnostics();
}

[Theory, WorkItem(61361, "https://github.com/dotnet/roslyn/issues/61361")]
[InlineData("==")]
[InlineData("!=")]
public void LiftedRelationalOperation_Equality(string op)
{
var comp = CreateCompilation($$"""
#nullable enable
class C
{
public static void Test(int? first, int? second)
{
if (first {{op}} second)
{
first.Value.ToString(); // 1
second.Value.ToString(); // 2
}

_ = (first {{op}} second)
? (first.Value.ToString() + second.Value.ToString()) // 3, 4
: string.Empty;

_ = (first {{op}} second) switch
{
true => first.Value.ToString() + second.Value.ToString(), // 5, 6
_ => string.Empty
};
}
}
""");
comp.VerifyDiagnostics(
// (8,13): warning CS8629: Nullable value type may be null.
// first.Value.ToString(); // 1
Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "first").WithLocation(8, 13),
// (9,13): warning CS8629: Nullable value type may be null.
// second.Value.ToString(); // 2
Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "second").WithLocation(9, 13),
// (13,16): warning CS8629: Nullable value type may be null.
// ? (first.Value.ToString() + second.Value.ToString()) // 3, 4
Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "first").WithLocation(13, 16),
// (13,41): warning CS8629: Nullable value type may be null.
// ? (first.Value.ToString() + second.Value.ToString()) // 3, 4
Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "second").WithLocation(13, 41),
// (18,21): warning CS8629: Nullable value type may be null.
// true => first.Value.ToString() + second.Value.ToString(), // 5, 6
Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "first").WithLocation(18, 21),
// (18,46): warning CS8629: Nullable value type may be null.
// true => first.Value.ToString() + second.Value.ToString(), // 5, 6
Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "second").WithLocation(18, 46)
);
}

[Fact, WorkItem(61361, "https://github.com/dotnet/roslyn/issues/61361")]
public void LiftedRelationalOperation_DateTime()
{
var comp = CreateCompilation(@"
using System;

#nullable enable
class C
{
public static DateTime Test(DateTime? first, DateTime? second, DateTime? third)
{
if (first < third)
{
if (second < first.Value)
{
}
}

return DateTime.Now;
}
}
");
comp.VerifyDiagnostics();
}
}
}

0 comments on commit 415427a

Please sign in to comment.