Skip to content

Commit b0726e2

Browse files
authored
Add support for nullable with runtime async (#82516)
The nullable walker was missing analysis of the runtime async helper, which caused several warnings to be missing. This adds support for those missing warnings. Relates to test plan #75960
1 parent 65e7dcf commit b0726e2

File tree

7 files changed

+485
-22
lines changed

7 files changed

+485
-22
lines changed

src/Compilers/CSharp/Portable/BoundTree/BoundTreeVisitors.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ protected BoundNode VisitExpressionOrPatternWithStackGuard(ref int recursionDept
224224
return result;
225225
}
226226

227+
[DebuggerStepThrough]
227228
protected virtual void EnsureSufficientExecutionStack(int recursionDepth)
228229
{
229230
StackGuard.EnsureSufficientExecutionStack(recursionDepth);

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ protected override BoundNode VisitExpressionOrPatternWithoutStackGuard(BoundNode
374374
return base.Visit(node);
375375
}
376376

377+
[DebuggerStepThrough]
377378
protected override bool ConvertInsufficientExecutionStackExceptionToCancelledByStackGuardException()
378379
{
379380
return false; // just let the original exception bubble up.

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

Lines changed: 90 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,7 @@ public string GetDebuggerDisplay()
529529
// For purpose of nullability analysis, awaits create pending branches, so async usings and foreachs do too
530530
public sealed override bool AwaitUsingAndForeachAddsPendingBranch => true;
531531

532+
[DebuggerStepThrough]
532533
protected override void EnsureSufficientExecutionStack(int recursionDepth)
533534
{
534535
if (recursionDepth > StackGuard.MaxUncheckedRecursionDepth &&
@@ -542,6 +543,7 @@ protected override void EnsureSufficientExecutionStack(int recursionDepth)
542543
base.EnsureSufficientExecutionStack(recursionDepth);
543544
}
544545

546+
[DebuggerStepThrough]
545547
protected override bool ConvertInsufficientExecutionStackExceptionToCancelledByStackGuardException()
546548
{
547549
return true;
@@ -595,7 +597,6 @@ private static void AssertPlaceholderAllowedWithoutRegistration(BoundValuePlaceh
595597
case BoundKind.InterpolatedStringHandlerPlaceholder:
596598
case BoundKind.InterpolatedStringArgumentPlaceholder:
597599
case BoundKind.ObjectOrCollectionValuePlaceholder:
598-
case BoundKind.AwaitableValuePlaceholder:
599600
return;
600601

601602
case BoundKind.ImplicitIndexerValuePlaceholder:
@@ -3598,16 +3599,43 @@ private void VisitLocalFunctionUse(LocalFunctionSymbol symbol)
35983599
public override BoundNode? VisitUsingStatement(BoundUsingStatement node)
35993600
{
36003601
DeclareLocals(node.Locals);
3601-
Visit(node.AwaitOpt);
3602+
VisitAwaitableInfoForUsing(node.AwaitOpt, node.PatternDisposeInfoOpt?.Method);
36023603
return base.VisitUsingStatement(node);
36033604
}
36043605

36053606
public override BoundNode? VisitUsingLocalDeclarations(BoundUsingLocalDeclarations node)
36063607
{
3607-
Visit(node.AwaitOpt);
3608+
VisitAwaitableInfoForUsing(node.AwaitOpt, node.PatternDisposeInfoOpt?.Method);
36083609
return base.VisitUsingLocalDeclarations(node);
36093610
}
36103611

3612+
private void VisitAwaitableInfoForUsing(BoundAwaitableInfo? awaitInfo, MethodSymbol? patternDisposeMethod)
3613+
{
3614+
// Placeholder can be null in error scenarios. In such cases, nothing is filled in and errors would
3615+
// have been reported already. We can just return.
3616+
if (awaitInfo is not { AwaitableInstancePlaceholder: { } placeholder })
3617+
{
3618+
return;
3619+
}
3620+
3621+
VisitResult placeholderResult;
3622+
if (patternDisposeMethod is not null)
3623+
{
3624+
placeholderResult = new VisitResult(GetReturnTypeWithState(patternDisposeMethod), patternDisposeMethod.ReturnTypeWithAnnotations);
3625+
}
3626+
else
3627+
{
3628+
// IAsyncDisposable.DisposeAsync returns ValueTask, which is a struct; we know it can never be `ValueTask?`, as that is a different
3629+
// type that would not match the descriptor. Any other scenario either has an error already, or will report an
3630+
// error during emit when we fail to find IAsyncDisposable.DisposeAsync.
3631+
placeholderResult = new VisitResult(placeholder.Type, NullableAnnotation.NotAnnotated, NullableFlowState.NotNull);
3632+
}
3633+
3634+
AddPlaceholderReplacement(placeholder, placeholder, placeholderResult);
3635+
Visit(awaitInfo);
3636+
RemovePlaceholderReplacement(placeholder);
3637+
}
3638+
36113639
public override BoundNode? VisitFixedStatement(BoundFixedStatement node)
36123640
{
36133641
DeclareLocals(node.Locals);
@@ -12069,23 +12097,13 @@ private void VisitForEachExpression(
1206912097
// Analyze `await DisposeAsync()`
1207012098
if (enumeratorInfoOpt is { NeedsDisposal: true, DisposeAwaitableInfo: BoundAwaitableInfo awaitDisposalInfo })
1207112099
{
12072-
var disposalPlaceholder = awaitDisposalInfo.AwaitableInstancePlaceholder;
12073-
bool addedPlaceholder = false;
12074-
if (enumeratorInfoOpt.PatternDisposeInfo is { Method: var originalDisposeMethod }) // no statically known Dispose method if doing a runtime check
12100+
var patternDisposeMethod = enumeratorInfoOpt.PatternDisposeInfo?.Method;
12101+
if (patternDisposeMethod is not null)
1207512102
{
12076-
Debug.Assert(disposalPlaceholder is not null);
12077-
var disposeAsyncMethod = (MethodSymbol)AsMemberOfType(reinferredGetEnumeratorMethod.ReturnType, originalDisposeMethod);
12078-
var result = new VisitResult(GetReturnTypeWithState(disposeAsyncMethod), disposeAsyncMethod.ReturnTypeWithAnnotations);
12079-
AddPlaceholderReplacement(disposalPlaceholder, disposalPlaceholder, result);
12080-
addedPlaceholder = true;
12103+
patternDisposeMethod = (MethodSymbol)AsMemberOfType(reinferredGetEnumeratorMethod.ReturnType, patternDisposeMethod);
1208112104
}
1208212105

12083-
Visit(awaitDisposalInfo);
12084-
12085-
if (addedPlaceholder)
12086-
{
12087-
RemovePlaceholderReplacement(disposalPlaceholder!);
12088-
}
12106+
VisitAwaitableInfoForUsing(awaitDisposalInfo, patternDisposeMethod);
1208912107
}
1209012108
}
1209112109

@@ -12571,7 +12589,12 @@ protected override void AfterLeftChildOfBinaryLogicalOperatorHasBeenVisited(Boun
1257112589
Visit(awaitableInfo);
1257212590
RemovePlaceholderReplacement(placeholder);
1257312591

12574-
if (node.Type.IsValueType || node.HasErrors || awaitableInfo.GetResult is null)
12592+
if (awaitableInfo is { GetResult: null, RuntimeAsyncAwaitCall: not null })
12593+
{
12594+
// This is AsyncHelpers.Await. We can directly use `_visitResult`
12595+
SetResult(node, _visitResult, updateAnalyzedNullability: true, isLvalue: false);
12596+
}
12597+
else if (node.Type.IsValueType || node.HasErrors || awaitableInfo.GetResult is null)
1257512598
{
1257612599
SetNotNullResult(node);
1257712600
}
@@ -13212,8 +13235,6 @@ private void VisitThrow(BoundExpression? expr)
1321213235

1321313236
public override BoundNode? VisitAwaitableValuePlaceholder(BoundAwaitableValuePlaceholder node)
1321413237
{
13215-
// These placeholders don't always follow proper placeholder discipline yet
13216-
AssertPlaceholderAllowedWithoutRegistration(node);
1321713238
VisitPlaceholderWithReplacement(node);
1321813239
return null;
1321913240
}
@@ -13235,8 +13256,55 @@ private void VisitPlaceholderWithReplacement(BoundValuePlaceholderBase node)
1323513256

1323613257
public override BoundNode? VisitAwaitableInfo(BoundAwaitableInfo node)
1323713258
{
13238-
Visit(node.AwaitableInstancePlaceholder);
13239-
Visit(node.GetAwaiter);
13259+
if (node is { GetAwaiter: null, GetResult: null, RuntimeAsyncAwaitCall: null, RuntimeAsyncAwaitCallPlaceholder: null })
13260+
{
13261+
if (node.AwaitableInstancePlaceholder is not null)
13262+
{
13263+
// Ensure that the placeholder has an analyzed nullability for the rewriter
13264+
SetNotNullResult(node.AwaitableInstancePlaceholder);
13265+
}
13266+
13267+
SetInvalidResult();
13268+
13269+
// Error scenario
13270+
return null;
13271+
}
13272+
13273+
Debug.Assert(node.AwaitableInstancePlaceholder is not null);
13274+
13275+
if (node.RuntimeAsyncAwaitCall is not null)
13276+
{
13277+
Debug.Assert(node.RuntimeAsyncAwaitCallPlaceholder is not null);
13278+
13279+
if (node.GetAwaiter is null)
13280+
{
13281+
// This is the simple case of just `AsyncHelpers.Await(expr)`. AwaitableInstancePlaceholder and RuntimeAsyncAwaitCallPlaceholder
13282+
// refer to the same value.
13283+
Debug.Assert(node.GetResult is null);
13284+
VisitPlaceholderWithReplacement(node.AwaitableInstancePlaceholder);
13285+
AddPlaceholderReplacement(node.RuntimeAsyncAwaitCallPlaceholder, node.AwaitableInstancePlaceholder, _visitResult);
13286+
Visit(node.RuntimeAsyncAwaitCall);
13287+
RemovePlaceholderReplacement(node.RuntimeAsyncAwaitCallPlaceholder);
13288+
}
13289+
else
13290+
{
13291+
// More complicated case of `AsyncHelpers.AwaitAwaiter/UnsafeAwaitAwaiter`. We need to visit GetAwaiter first to get the nullability
13292+
// result for the awaiter, which we then save. It is used both here, as the state of the RuntimeAsyncAwaitCallPlaceholder replacement,
13293+
// and also for the caller to be able to re-infer GetResult with nullability.
13294+
Visit(node.GetAwaiter);
13295+
var getAwaiterResult = _visitResult;
13296+
AddPlaceholderReplacement(node.RuntimeAsyncAwaitCallPlaceholder, node.GetAwaiter, getAwaiterResult);
13297+
Visit(node.RuntimeAsyncAwaitCall);
13298+
RemovePlaceholderReplacement(node.RuntimeAsyncAwaitCallPlaceholder);
13299+
_visitResult = getAwaiterResult;
13300+
}
13301+
}
13302+
else
13303+
{
13304+
// Not runtime async
13305+
Visit(node.GetAwaiter);
13306+
}
13307+
1324013308
return null;
1324113309
}
1324213310

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncTests.cs

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10883,5 +10883,188 @@ .maxstack 1
1088310883
}
1088410884
""");
1088510885
}
10886+
10887+
[Fact]
10888+
public void RuntimeAsyncNullable_WithTaskReturningMethod()
10889+
{
10890+
var source = """
10891+
using System.Threading.Tasks;
10892+
10893+
#nullable enable
10894+
10895+
await M(null);
10896+
10897+
async Task? M(object o) { }
10898+
""";
10899+
10900+
var comp = CreateRuntimeAsyncCompilation(source);
10901+
var verifier = CompileAndVerify(comp, verify: Verification.Fails with
10902+
{
10903+
ILVerifyMessage = $"""
10904+
{ReturnValueMissing("<Main>$", "0xb")}
10905+
{ReturnValueMissing("<<Main>$>g__M|0_0", "0x0")}
10906+
"""
10907+
});
10908+
verifier.VerifyDiagnostics(
10909+
// (5,7): warning CS8604: Possible null reference argument for parameter 'task' in 'void AsyncHelpers.Await(Task task)'.
10910+
// await M(null);
10911+
Diagnostic(ErrorCode.WRN_NullReferenceArgument, "M(null)").WithArguments("task", "void AsyncHelpers.Await(Task task)").WithLocation(5, 7),
10912+
// (5,9): warning CS8625: Cannot convert null literal to non-nullable reference type.
10913+
// await M(null);
10914+
Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(5, 9)
10915+
);
10916+
}
10917+
10918+
[Fact]
10919+
public void RuntimeAsyncNullable_WithTaskLikeReturningMethod()
10920+
{
10921+
var corlib = """
10922+
namespace System
10923+
{
10924+
public class Attribute {}
10925+
public enum AttributeTargets {}
10926+
public class AttributeUsageAttribute : Attribute
10927+
{
10928+
public AttributeUsageAttribute(AttributeTargets validOn) {}
10929+
public bool AllowMultiple { get; set; }
10930+
public bool Inherited { get; set; }
10931+
}
10932+
public struct Boolean {}
10933+
public struct Byte {}
10934+
public abstract class Enum {}
10935+
public class Exception {}
10936+
public struct Int32 {}
10937+
public class Object {}
10938+
public class String {}
10939+
public class ValueType {}
10940+
public class Void {}
10941+
10942+
namespace Threading.Tasks
10943+
{
10944+
public class Task
10945+
{
10946+
public Runtime.CompilerServices.TaskAwaiter GetAwaiter() => throw null!;
10947+
public static Task CompletedTask => throw null!;
10948+
}
10949+
}
10950+
10951+
namespace Runtime.CompilerServices
10952+
{
10953+
public interface INotifyCompletion {}
10954+
public interface ICriticalNotifyCompletion : INotifyCompletion {}
10955+
public class TaskAwaiter : ICriticalNotifyCompletion
10956+
{
10957+
public bool IsCompleted => false;
10958+
public void GetResult() {}
10959+
}
10960+
10961+
public static class AsyncHelpers
10962+
{
10963+
public static void UnsafeAwaitAwaiter<TAwaiter>(TAwaiter awaiter) where TAwaiter : notnull, INotifyCompletion {}
10964+
}
10965+
}
10966+
}
10967+
""";
10968+
10969+
var source = """
10970+
#nullable enable
10971+
10972+
await M(null);
10973+
10974+
MyTask M(object o) => throw null!;
10975+
10976+
public class MyTask
10977+
{
10978+
public System.Runtime.CompilerServices.TaskAwaiter? GetAwaiter() => throw null!;
10979+
}
10980+
""";
10981+
10982+
var comp = CreateEmptyCompilation([source, corlib], parseOptions: WithRuntimeAsync(TestOptions.RegularPreview), assemblyName: "TestAssembly");
10983+
var verifier = CompileAndVerify(comp, verify: Verification.Skipped);
10984+
verifier.VerifyDiagnostics(
10985+
// warning CS8021: No value for RuntimeMetadataVersion found. No assembly containing System.Object was found nor was a value for RuntimeMetadataVersion specified through options.
10986+
Diagnostic(ErrorCode.WRN_NoRuntimeMetadataVersion).WithLocation(1, 1),
10987+
// (3,7): warning CS8714: The type 'System.Runtime.CompilerServices.TaskAwaiter?' cannot be used as type parameter 'TAwaiter' in the generic type or method 'AsyncHelpers.UnsafeAwaitAwaiter<TAwaiter>(TAwaiter)'. Nullability of type argument 'System.Runtime.CompilerServices.TaskAwaiter?' doesn't match 'notnull' constraint.
10988+
// await M(null);
10989+
Diagnostic(ErrorCode.WRN_NullabilityMismatchInTypeParameterNotNullConstraint, "M").WithArguments("System.Runtime.CompilerServices.AsyncHelpers.UnsafeAwaitAwaiter<TAwaiter>(TAwaiter)", "TAwaiter", "System.Runtime.CompilerServices.TaskAwaiter?").WithLocation(3, 7),
10990+
// (3,9): warning CS8625: Cannot convert null literal to non-nullable reference type.
10991+
// await M(null);
10992+
Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(3, 9)
10993+
);
10994+
}
10995+
10996+
[Theory]
10997+
[InlineData("Task")]
10998+
[InlineData("ValueTask")]
10999+
public void RuntimeAsyncAwaitNullableFlow_WithTaskReturningMethod(string taskType)
11000+
{
11001+
var source = $$"""
11002+
using System.Threading.Tasks;
11003+
11004+
#nullable enable
11005+
11006+
_ = (await GetNullableResultAsync()).ToString();
11007+
_ = (await GetNullableResultAsync().ConfigureAwait(true)).ToString();
11008+
var x = await GetNullableResultAsync();
11009+
_ = x.ToString(); // 1
11010+
x = await GetNullableResultAsync().ConfigureAwait(false);
11011+
_ = x.ToString(); // 2
11012+
11013+
{{taskType}}<string?> GetNullableResultAsync() => default!;
11014+
""";
11015+
11016+
var comp = CreateRuntimeAsyncCompilation(source);
11017+
comp.VerifyDiagnostics(
11018+
// (5,6): warning CS8602: Dereference of a possibly null reference.
11019+
// _ = (await GetNullableResultAsync()).ToString();
11020+
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "await GetNullableResultAsync()").WithLocation(5, 6),
11021+
// (6,6): warning CS8602: Dereference of a possibly null reference.
11022+
// _ = (await GetNullableResultAsync().ConfigureAwait(true)).ToString();
11023+
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "await GetNullableResultAsync().ConfigureAwait(true)").WithLocation(6, 6),
11024+
// (8,5): warning CS8602: Dereference of a possibly null reference.
11025+
// _ = x.ToString(); // 1
11026+
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(8, 5),
11027+
// (10,5): warning CS8602: Dereference of a possibly null reference.
11028+
// _ = x.ToString(); // 2
11029+
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(10, 5)
11030+
);
11031+
}
11032+
11033+
[Fact]
11034+
public void RuntimeAsyncAwaitNullableFlow_WithTaskLikeReturningMethod()
11035+
{
11036+
var source = """
11037+
#nullable enable
11038+
11039+
_ = (await GetNullableResultTaskLike()).ToString();
11040+
var x = await GetNullableResultTaskLike();
11041+
_ = x.ToString();
11042+
11043+
MyTask GetNullableResultTaskLike() => throw null!;
11044+
11045+
public class MyTask
11046+
{
11047+
public MyAwaiter GetAwaiter() => throw null!;
11048+
}
11049+
11050+
public class MyAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion
11051+
{
11052+
public bool IsCompleted => false;
11053+
public string? GetResult() => null;
11054+
public void OnCompleted(System.Action continuation) { }
11055+
public void UnsafeOnCompleted(System.Action continuation) { }
11056+
}
11057+
""";
11058+
11059+
var comp = CreateRuntimeAsyncCompilation(source);
11060+
comp.VerifyDiagnostics(
11061+
// (3,6): warning CS8602: Dereference of a possibly null reference.
11062+
// _ = (await GetNullableResultTaskLike()).ToString();
11063+
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "await GetNullableResultTaskLike()").WithLocation(3, 6),
11064+
// (5,5): warning CS8602: Dereference of a possibly null reference.
11065+
// _ = x.ToString();
11066+
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(5, 5)
11067+
);
11068+
}
1088611069
}
1088711070
}

0 commit comments

Comments
 (0)