-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Move runtime async method validation into initial binding #78310
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
Changes from all commits
351c386
442f9f8
216d5c9
ff721e5
1f5aefc
33a4423
7918319
9df8371
c10918c
459f528
0e5c942
fa1b1d9
20273bb
5a789ef
3bb7189
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.Diagnostics; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.CSharp; | ||
|
|
||
| partial class BoundAwaitableInfo | ||
| { | ||
| private partial void Validate() | ||
| { | ||
| if (RuntimeAsyncAwaitMethod is not null) | ||
| { | ||
| Debug.Assert(RuntimeAsyncAwaitMethod.ContainingType.ExtendedSpecialType == InternalSpecialType.System_Runtime_CompilerServices_AsyncHelpers); | ||
|
|
||
| switch (RuntimeAsyncAwaitMethod.Name) | ||
| { | ||
| case "Await": | ||
| Debug.Assert(GetAwaiter is null); | ||
| Debug.Assert(IsCompleted is null); | ||
| Debug.Assert(GetResult is null); | ||
| break; | ||
|
|
||
| case "AwaitAwaiter": | ||
| case "UnsafeAwaitAwaiter": | ||
| Debug.Assert(GetAwaiter is not null); | ||
| Debug.Assert(IsCompleted is not null); | ||
| Debug.Assert(GetResult is not null); | ||
| break; | ||
|
|
||
| default: | ||
| Debug.Fail($"Unexpected RuntimeAsyncAwaitMethod: {RuntimeAsyncAwaitMethod.Name}"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checking my understanding: it looks like we expect this condition to never occur. i.e. compiler will never produce a
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, that's correct. |
||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -316,7 +316,7 @@ internal bool IsNullableAnalysisEnabledAlways | |
| /// Returns true if this method should be processed with runtime async handling instead | ||
| /// of compiler async state machine generation. | ||
| /// </summary> | ||
| internal bool IsRuntimeAsyncEnabledIn(MethodSymbol method) | ||
| internal bool IsRuntimeAsyncEnabledIn(Symbol? symbol) | ||
| { | ||
| // PROTOTYPE: EE tests fail this assert, handle and test | ||
| //Debug.Assert(ReferenceEquals(method.ContainingAssembly, Assembly)); | ||
|
|
@@ -325,7 +325,22 @@ internal bool IsRuntimeAsyncEnabledIn(MethodSymbol method) | |
| return false; | ||
| } | ||
|
|
||
| return method switch | ||
| if (symbol is not MethodSymbol method) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| var methodReturn = method.ReturnType.OriginalDefinition; | ||
| if (((InternalSpecialType)methodReturn.ExtendedSpecialType) is not ( | ||
| InternalSpecialType.System_Threading_Tasks_Task or | ||
| InternalSpecialType.System_Threading_Tasks_Task_T or | ||
| InternalSpecialType.System_Threading_Tasks_ValueTask or | ||
| InternalSpecialType.System_Threading_Tasks_ValueTask_T)) | ||
| { | ||
| return false; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a corresponding update to the design doc? Or should we have a follow-up comment to make the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to update the design doc, the runtime-side is very clear that only Task/ValueTask methods can be runtime async. |
||
| } | ||
|
|
||
| return symbol switch | ||
| { | ||
| SourceMethodSymbol { IsRuntimeAsyncEnabledInMethod: ThreeState.True } => true, | ||
| SourceMethodSymbol { IsRuntimeAsyncEnabledInMethod: ThreeState.False } => false, | ||
|
|
@@ -2210,12 +2225,17 @@ internal bool ReturnsAwaitableToVoidOrInt(MethodSymbol method, BindingDiagnostic | |
| var syntax = method.ExtractReturnTypeSyntax(); | ||
| var dumbInstance = new BoundLiteral(syntax, ConstantValue.Null, namedType); | ||
| var binder = GetBinder(syntax); | ||
| BoundExpression? result; | ||
| var success = binder.GetAwaitableExpressionInfo(dumbInstance, out result, syntax, diagnostics); | ||
| var success = binder.GetAwaitableExpressionInfo(dumbInstance, out BoundExpression? result, out MethodSymbol? runtimeAwaitMethod, syntax, diagnostics); | ||
|
|
||
| RoslynDebug.Assert(!namedType.IsDynamic()); | ||
| return success && | ||
| (result!.Type!.IsVoidType() || result.Type!.SpecialType == SpecialType.System_Int32); | ||
| if (!success) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| Debug.Assert(result is { Type: not null } || runtimeAwaitMethod is { ReturnType: not null }); | ||
| var returnType = result?.Type ?? runtimeAwaitMethod!.ReturnType; | ||
| return returnType.IsVoidType() || returnType.SpecialType == SpecialType.System_Int32; | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking my understanding on the sequence of checks here.
Is the idea that in the 'RuntimeAwaitAwaiter' case, we still need to use the type's GetAwaiter method etc? That seems to make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Precisely. This the lowering mechanism in https://github.com/dotnet/roslyn/blob/main/docs/compilers/CSharp/Runtime%20Async%20Design.md#await-any-other-type, as opposed to the rest of the document which talks about
await Task