Add RuntimeAwaitMethod to AwaitExpressionInfo #79968
Conversation
|
@dotnet/roslyn-compiler for review |
dotnet-policy-service
bot
added
the
Needs API Review
| /// When runtime async is enabled for this await expression, this represents either: | ||
| /// <list type="bullet"> | ||
| /// <item> | ||
| /// A call to <c>System.Runtime.CompilerServices.AsyncHelpers.Await</c>, if this is a |
There was a problem hiding this comment.
This is not a "call", it's a method symbol
| /// </item> | ||
| /// <item> | ||
| /// A call to <c>System.Runtime.CompilerServices.AsyncHelpers.AwaitAwaiter|UnsafeAwaitAwaiter</c>. | ||
| /// In these cases, the other properties may be non-<see langword="null" /> if the |
There was a problem hiding this comment.
I assume you mean that GetAwaiterMethod/IsCompletedProperty/GetResultMethod may be set in runtime-async scenario. Looking at the logic in GetAwaitableExpressionInfo, we don't set those in runtime-async scenario (since we won't need them). Why not make this behavior official in the public API? The behavior we have seems better than the behavior this comment describes. #Closed
There was a problem hiding this comment.
Looking at the logic in GetAwaitableExpressionInfo, we don't set those in runtime-async scenario (since we won't need them).
We do set them in this case, because we will need them.
There was a problem hiding this comment.
Yup, I misread. We don't need the other members for System.Runtime.CompilerServices.AsyncHelpers.Await case, but we do need them for the AwaitAwiter/UnsafeAwaitAwaiter cases. Thanks
| GetAwaiterMethod = getAwaiter; | ||
| IsCompletedProperty = isCompleted; | ||
| GetResultMethod = getResult; | ||
| RuntimeAwaitMethod = runtimeAwaitMethod; |
There was a problem hiding this comment.
Consider asserting that we either set runtimeAwaitMethod or the members related to non-runtime-async #Closed
There was a problem hiding this comment.
This is not a thing that can be asserted.
| AssertEx.Equal("void System.Runtime.CompilerServices.YieldAwaitable.YieldAwaiter.GetResult()", info.GetResultMethod.ToTestDisplayString()); | ||
| AssertEx.Equal("System.Boolean System.Runtime.CompilerServices.YieldAwaitable.YieldAwaiter.IsCompleted { get; }", info.IsCompletedProperty.ToTestDisplayString()); | ||
| AssertEx.Equal( | ||
| "void System.Runtime.CompilerServices.AsyncHelpers.UnsafeAwaitAwaiter<System.Runtime.CompilerServices.YieldAwaitable.YieldAwaiter>(System.Runtime.CompilerServices.YieldAwaitable.YieldAwaiter awaiter)", |
There was a problem hiding this comment.
Consider also testing a scenario where RuntimeAwaitMethod is AwaitAwaiter instead of UnsafeAwaitAwaiter.
5 participants
Closes #79818.