Skip to content

Commit

Permalink
Small nullability fixes (#49279)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv authored Nov 14, 2020
1 parent bd3e3a2 commit a341062
Show file tree
Hide file tree
Showing 3 changed files with 219 additions and 26 deletions.
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ private void VerifyExpression(BoundExpression expression, bool overrideSkippedEx
{
Visit(node.IterationVariableType);
Visit(node.AwaitOpt);
Visit(node.EnumeratorInfoOpt?.DisposeAwaitableInfo);
Visit(node.Expression);
// https://github.com/dotnet/roslyn/issues/35010: handle the deconstruction
//this.Visit(node.DeconstructionOpt);
Expand Down
66 changes: 53 additions & 13 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2739,7 +2739,6 @@ private void VisitLocalFunctionUse(LocalFunctionSymbol symbol)
public override BoundNode? VisitForEachStatement(BoundForEachStatement node)
{
DeclareLocals(node.IterationVariables);
Visit(node.AwaitOpt);
return base.VisitForEachStatement(node);
}

Expand Down Expand Up @@ -5283,6 +5282,17 @@ private void VisitArgumentConversionAndInboundAssignmentsAndPreConditions(
case RefKind.In:
{
// Note: for lambda arguments, they will be converted in the context/state we saved for that argument
if (conversion is { Kind: ConversionKind.ImplicitUserDefined })
{
var argumentResultType = resultType.Type;
conversion = GenerateConversion(_conversions, argumentNoConversion, argumentResultType, parameterType.Type, fromExplicitCast: false, extensionMethodThisArgument: false);
if (!conversion.Exists && !argumentNoConversion.IsSuppressed)
{
Debug.Assert(argumentResultType is not null);
ReportNullabilityMismatchInArgument(argumentNoConversion.Syntax, argumentResultType, parameter, parameterType.Type, forOutput: false);
}
}

var stateAfterConversion = VisitConversion(
conversionOpt: conversionOpt,
conversionOperand: argumentNoConversion,
Expand Down Expand Up @@ -6315,17 +6325,6 @@ private void TrackNullableStateOfNullableValue(int containingSlot, TypeSymbol co
}
}

private void TrackNullableStateOfNullableValue(BoundExpression node, BoundExpression operand, TypeSymbol convertedType, TypeWithAnnotations underlyingType)
{
int valueSlot = MakeSlot(operand);
if (valueSlot > 0)
{
int containingSlot = GetOrCreatePlaceholderSlot(node);
Debug.Assert(containingSlot > 0);
TrackNullableStateOfNullableValue(containingSlot, convertedType, operand, underlyingType.ToTypeWithState(), valueSlot);
}
}

private void TrackNullableStateOfTupleConversion(
BoundConversion? conversionOpt,
BoundExpression convertedNode,
Expand Down Expand Up @@ -8495,6 +8494,41 @@ protected override void VisitForEachExpression(BoundForEachStatement node)
currentPropertyGetterTypeWithState = ApplyUnconditionalAnnotations(
currentPropertyGetter.ReturnTypeWithAnnotations.ToTypeWithState(),
currentPropertyGetter.ReturnTypeFlowAnalysisAnnotations);

// Analyze `await MoveNextAsync()`
if (node.AwaitOpt is { AwaitableInstancePlaceholder: BoundAwaitableValuePlaceholder moveNextPlaceholder } awaitMoveNextInfo)
{
var moveNextAsyncMethod = (MethodSymbol)AsMemberOfType(reinferredGetEnumeratorMethod.ReturnType, node.EnumeratorInfoOpt.MoveNextMethod);

EnsureAwaitablePlaceholdersInitialized();
var result = new VisitResult(GetReturnTypeWithState(moveNextAsyncMethod), moveNextAsyncMethod.ReturnTypeWithAnnotations);
_awaitablePlaceholdersOpt.Add(moveNextPlaceholder, (moveNextPlaceholder, result));
Visit(awaitMoveNextInfo);
_awaitablePlaceholdersOpt.Remove(moveNextPlaceholder);
}

// Analyze `await DisposeAsync()`
if (node.EnumeratorInfoOpt is { NeedsDisposal: true, DisposeAwaitableInfo: BoundAwaitableInfo awaitDisposalInfo })
{
var disposalPlaceholder = awaitDisposalInfo.AwaitableInstancePlaceholder;
bool addedPlaceholder = false;
if (node.EnumeratorInfoOpt.DisposeMethod is not null) // no statically known Dispose method if doing a runtime check
{
Debug.Assert(disposalPlaceholder is not null);
var disposeAsyncMethod = (MethodSymbol)AsMemberOfType(reinferredGetEnumeratorMethod.ReturnType, node.EnumeratorInfoOpt.DisposeMethod);
EnsureAwaitablePlaceholdersInitialized();
var result = new VisitResult(GetReturnTypeWithState(disposeAsyncMethod), disposeAsyncMethod.ReturnTypeWithAnnotations);
_awaitablePlaceholdersOpt.Add(disposalPlaceholder, (disposalPlaceholder, result));
addedPlaceholder = true;
}

Visit(awaitDisposalInfo);

if (addedPlaceholder)
{
_awaitablePlaceholdersOpt!.Remove(disposalPlaceholder!);
}
}
}

SetResultType(expression: null, currentPropertyGetterTypeWithState);
Expand Down Expand Up @@ -8845,7 +8879,7 @@ private TypeWithState InferResultNullabilityOfBinaryLogicalOperator(BoundExpress
var placeholder = awaitableInfo.AwaitableInstancePlaceholder;
Debug.Assert(placeholder is object);

_awaitablePlaceholdersOpt ??= PooledDictionary<BoundAwaitableValuePlaceholder, (BoundExpression AwaitableExpression, VisitResult Result)>.GetInstance();
EnsureAwaitablePlaceholdersInitialized();
_awaitablePlaceholdersOpt.Add(placeholder, (node.Expression, _visitResult));
Visit(awaitableInfo);
_awaitablePlaceholdersOpt.Remove(placeholder);
Expand Down Expand Up @@ -9436,6 +9470,12 @@ protected override void VisitCatchBlock(BoundCatchBlock node, ref LocalState fin
return null;
}

[MemberNotNull(nameof(_awaitablePlaceholdersOpt))]
private void EnsureAwaitablePlaceholdersInitialized()
{
_awaitablePlaceholdersOpt ??= PooledDictionary<BoundAwaitableValuePlaceholder, (BoundExpression AwaitableExpression, VisitResult Result)>.GetInstance();
}

public override BoundNode? VisitAwaitableValuePlaceholder(BoundAwaitableValuePlaceholder node)
{
if (_awaitablePlaceholdersOpt != null && _awaitablePlaceholdersOpt.TryGetValue(node, out var value))
Expand Down
178 changes: 165 additions & 13 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60764,7 +60764,7 @@ static void G(A<string> a2, B b2)
Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "b2").WithArguments("B", "IOut<object>").WithLocation(19, 13));
}

[Fact]
[Fact, WorkItem(29898, "https://github.com/dotnet/roslyn/issues/29898")]
public void ImplicitConversions_07()
{
var source =
Expand All @@ -60784,13 +60784,107 @@ static void Main(object? x)
var y = F(x);
G(y);
if (x == null) return;
var z = F(x);
var z = F(x);
G(z); // warning

var z2 = F(x);
G(z2!);
}
}";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
// https://github.com/dotnet/roslyn/issues/29898: Report warning for `G(z)`?
comp.VerifyDiagnostics();
comp.VerifyDiagnostics(
// (18,11): warning CS8620: Argument of type 'B<object>' cannot be used for parameter 'a' of type 'A<object?>' in 'void C.G(A<object?> a)' due to differences in the nullability of reference types.
// G(z); // warning
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "z").WithArguments("B<object>", "A<object?>", "a", "void C.G(A<object?> a)").WithLocation(18, 11)
);
}

[Fact]
public void ImplicitConversion_Params()
{
var source =
@"class A<T>
{
}
class B<T>
{
public static implicit operator A<T>(B<T> b) => throw null!;
}
class C
{
static B<T> F<T>(T t) => throw null!;
static void G(params A<object>[] a) => throw null!;

static void Main(object? x)
{
var y = F(x);
G(y); // 1

if (x == null) return;
var z = F(x);
G(z);
}
}";
var comp = CreateCompilation(source, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (16,11): warning CS8620: Argument of type 'B<object?>' cannot be used for parameter 'a' of type 'A<object>' in 'void C.G(params A<object>[] a)' due to differences in the nullability of reference types.
// G(y); // 1
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "y").WithArguments("B<object?>", "A<object>", "a", "void C.G(params A<object>[] a)").WithLocation(16, 11)
);
}

[Fact]
public void ImplicitConversion_Typeless()
{
var source = @"
public struct Optional<T>
{
public static implicit operator Optional<T>(T value) => throw null!;
}

class C
{
static void G1(Optional<object> a) => throw null!;
static void G2(Optional<object?> a) => throw null!;

static void M()
{
G1(null); // 1
G2(null);
}
}";
var comp = CreateCompilation(source, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (14,12): warning CS8625: Cannot convert null literal to non-nullable reference type.
// G1(null); // 1
Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(14, 12)
);
}

[Fact]
public void ImplicitConversion_Typeless_WithConstraint()
{
var source = @"
public struct Optional<T> where T : class
{
public static implicit operator Optional<T>(T value) => throw null!;
}

class C
{
static void G(Optional<object> a) => throw null!;

static void M()
{
G(null);
}
}";
var comp = CreateCompilation(source, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (13,11): warning CS8625: Cannot convert null literal to non-nullable reference type.
// G(null);
Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(13, 11)
);
}

[Fact, WorkItem(41763, "https://github.com/dotnet/roslyn/issues/41763")]
Expand Down Expand Up @@ -82660,10 +82754,11 @@ static void F3((B?, B) t)
}";
var comp = CreateCompilation(source, options: WithNonNullTypesTrue());
// https://github.com/dotnet/roslyn/issues/32599: Handle tuple element conversions.
// Diagnostics on user-defined conversions need improvement: https://github.com/dotnet/roslyn/issues/31798
comp.VerifyDiagnostics(
// (16,12): warning CS8620: Argument of type '(A?, A)?' cannot be used for parameter 't' of type '(A, A?)?' in 'void Program.F2((A, A?)? t)' due to differences in the nullability of reference types.
// (16,12): warning CS8620: Argument of type 'S' cannot be used for parameter 't' of type '(A, A?)?' in 'void Program.F2((A, A?)? t)' due to differences in the nullability of reference types.
// F2(s); // 1
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "s").WithArguments("(A?, A)?", "(A, A?)?", "t", "void Program.F2((A, A?)? t)").WithLocation(16, 12));
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "s").WithArguments("S", "(A, A?)?", "t", "void Program.F2((A, A?)? t)").WithLocation(16, 12));
}

[Fact]
Expand Down Expand Up @@ -117089,9 +117184,9 @@ static void Main()
// (12,15): warning CS8619: Nullability of reference types in value of type 'A<object?>' doesn't match target type 'A<object>'.
// B b = a; // 1
Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "a").WithArguments("A<object?>", "A<object>").WithLocation(12, 15),
// (13,11): warning CS8620: Nullability of reference types in argument of type 'A<object?>' doesn't match target type 'A<object>' for parameter 'b' in 'void Program.F(B b)'.
// (13,11): warning CS8620: Argument of type 'A<object?>' cannot be used for parameter 'b' of type 'B' in 'void Program.F(B b)' due to differences in the nullability of reference types.
// F(a); // 2
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "a").WithArguments("A<object?>", "A<object>", "b", "void Program.F(B b)").WithLocation(13, 11));
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "a").WithArguments("A<object?>", "B", "b", "void Program.F(B b)").WithLocation(13, 11));
}

[Fact]
Expand Down Expand Up @@ -117119,9 +117214,9 @@ static void Main()
// (12,24): warning CS8619: Nullability of reference types in value of type 'A<object>' doesn't match target type 'A<object?>'.
// A<object?> a = b; // 1
Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "b").WithArguments("A<object>", "A<object?>").WithLocation(12, 24),
// (13,11): warning CS8620: Nullability of reference types in argument of type 'A<object>' doesn't match target type 'A<object?>' for parameter 'a' in 'void Program.F(A<object?> a)'.
// (13,11): warning CS8620: Argument of type 'B' cannot be used for parameter 'a' of type 'A<object?>' in 'void Program.F(A<object?> a)' due to differences in the nullability of reference types.
// F(b); // 2
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "b").WithArguments("A<object>", "A<object?>", "a", "void Program.F(A<object?> a)").WithLocation(13, 11));
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "b").WithArguments("B", "A<object?>", "a", "void Program.F(A<object?> a)").WithLocation(13, 11));
}

[WorkItem(31864, "https://github.com/dotnet/roslyn/issues/31864")]
Expand Down Expand Up @@ -132291,17 +132386,74 @@ static async Task Main()
await foreach (var o in Create(x)) // 2
{
}
await foreach (var o in Create(y))
await foreach (var o in Create(y)) // 3
{
}
}
}";
var comp = CreateCompilationWithTasksExtensions(new[] { s_IAsyncEnumerable, source });
// Should report warning for GetAwaiter().
comp.VerifyDiagnostics(
// (24,20): warning CS8600: Converting null literal or possible null value to non-nullable type.
// object y = null; // 1
Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "null").WithLocation(24, 20));
Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "null").WithLocation(24, 20),
// (25,33): warning CS8620: Argument of type 'StructAwaitable1<object>' cannot be used for parameter 's' of type 'StructAwaitable1<object?>' in 'TaskAwaiter<bool> Program.GetAwaiter(StructAwaitable1<object?> s)' due to differences in the nullability of reference types.
// await foreach (var o in Create(x)) // 2
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "Create(x)").WithArguments("StructAwaitable1<object>", "StructAwaitable1<object?>", "s", "TaskAwaiter<bool> Program.GetAwaiter(StructAwaitable1<object?> s)").WithLocation(25, 33),
// (28,33): warning CS8620: Argument of type 'StructAwaitable2<object?>' cannot be used for parameter 's' of type 'StructAwaitable2<object>' in 'TaskAwaiter Program.GetAwaiter(StructAwaitable2<object> s)' due to differences in the nullability of reference types.
// await foreach (var o in Create(y)) // 3
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "Create(y)").WithArguments("StructAwaitable2<object?>", "StructAwaitable2<object>", "s", "TaskAwaiter Program.GetAwaiter(StructAwaitable2<object> s)").WithLocation(28, 33)
);
}

[Fact]
[WorkItem(30956, "https://github.com/dotnet/roslyn/issues/30956")]
public void GetAwaiterExtensionMethod_AwaitForEach_InverseAnnotations()
{
var source =
@"#nullable enable
using System.Runtime.CompilerServices;
using System.Threading.Tasks;
struct StructAwaitable1<T> { }
struct StructAwaitable2<T> { }
class Enumerable<T>
{
public Enumerator<T> GetAsyncEnumerator() => new Enumerator<T>();
}
class Enumerator<T>
{
public object Current => null!;
public StructAwaitable1<T> MoveNextAsync() => new StructAwaitable1<T>();
public StructAwaitable2<T> DisposeAsync() => new StructAwaitable2<T>();
}
static class Program
{
static TaskAwaiter<bool> GetAwaiter(this StructAwaitable1<object> s) => default;
static TaskAwaiter GetAwaiter(this StructAwaitable2<object?> s) => default;
static Enumerable<T> Create<T>(T t) => new Enumerable<T>();
static async Task Main()
{
object? x = new object();
object y = null; // 1
await foreach (var o in Create(x)) // 2
{
}
await foreach (var o in Create(y)) // 3
{
}
}
}";
var comp = CreateCompilationWithTasksExtensions(new[] { s_IAsyncEnumerable, source });
comp.VerifyDiagnostics(
// (24,20): warning CS8600: Converting null literal or possible null value to non-nullable type.
// object y = null; // 1
Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "null").WithLocation(24, 20),
// (25,33): warning CS8620: Argument of type 'StructAwaitable2<object>' cannot be used for parameter 's' of type 'StructAwaitable2<object?>' in 'TaskAwaiter Program.GetAwaiter(StructAwaitable2<object?> s)' due to differences in the nullability of reference types.
// await foreach (var o in Create(x)) // 2
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "Create(x)").WithArguments("StructAwaitable2<object>", "StructAwaitable2<object?>", "s", "TaskAwaiter Program.GetAwaiter(StructAwaitable2<object?> s)").WithLocation(25, 33),
// (28,33): warning CS8620: Argument of type 'StructAwaitable1<object?>' cannot be used for parameter 's' of type 'StructAwaitable1<object>' in 'TaskAwaiter<bool> Program.GetAwaiter(StructAwaitable1<object> s)' due to differences in the nullability of reference types.
// await foreach (var o in Create(y)) // 3
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "Create(y)").WithArguments("StructAwaitable1<object?>", "StructAwaitable1<object>", "s", "TaskAwaiter<bool> Program.GetAwaiter(StructAwaitable1<object> s)").WithLocation(28, 33)
);
}

[Fact]
Expand Down

0 comments on commit a341062

Please sign in to comment.