Skip to content

Ensure return local is the correct type for runtime async #78603

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 19, 2025
Merged

Ensure return local is the correct type for runtime async #78603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
81b97a5
Add test demonstrating current behavior
333fred May 15, 2025
be59ccb
Ensure return local is the correct type in async scenarios
333fred May 15, 2025
8866bee
Ensure method is actually async when doing local rewrite
333fred May 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,24 @@ private LocalDefinition LazyReturnTemp
? LocalSlotConstraints.None
: LocalSlotConstraints.ByRef;

var returnTypeWithAnnotations = _method.ReturnTypeWithAnnotations;
if (_method.IsAsync && _module.Compilation.IsRuntimeAsyncEnabledIn(_method))
{
// The return type of the method is either Task<T> or ValueTask<T>. The il of the method is
// actually going to appear to return a T, not the wrapper task type. So we need to
// translate the return type to the actual type that will be returned.
Copy link
Member

@RikkiGibson RikkiGibson May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. The reason this change makes sense is: within an async method, return statements are supposed to return a T rather than Task<T>. For legacy async, we didn't actually generate a return temp of type T, because doing so wasn't sensible for the way that the legacy async codegen works. However, using T for the return temp is very sensible because we are just calling the runtime helper which unwraps the T out of the Task<T>.


var returnType = returnTypeWithAnnotations.Type;
Debug.Assert(((InternalSpecialType)returnType.OriginalDefinition.ExtendedSpecialType) is InternalSpecialType.System_Threading_Tasks_ValueTask_T or InternalSpecialType.System_Threading_Tasks_Task_T);

returnTypeWithAnnotations = ((NamedTypeSymbol)returnType).TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0];
}

var bodySyntax = _methodBodySyntaxOpt;
if (_ilEmitStyle == ILEmitStyle.Debug && bodySyntax != null)
{
int syntaxOffset = _method.CalculateLocalSyntaxOffset(LambdaUtilities.GetDeclaratorPosition(bodySyntax), bodySyntax.SyntaxTree);
var localSymbol = new SynthesizedLocal(_method, _method.ReturnTypeWithAnnotations, SynthesizedLocalKind.FunctionReturnValue, bodySyntax);
var localSymbol = new SynthesizedLocal(_method, returnTypeWithAnnotations, SynthesizedLocalKind.FunctionReturnValue, bodySyntax);

result = _builder.LocalSlotManager.DeclareLocal(
type: _module.Translate(localSymbol.Type, bodySyntax, _diagnostics.DiagnosticBag),
Expand All @@ -190,7 +203,7 @@ private LocalDefinition LazyReturnTemp
}
else
{
result = AllocateTemp(_method.ReturnType, _boundBody.Syntax, slotConstraints);
result = AllocateTemp(returnTypeWithAnnotations.Type, _boundBody.Syntax, slotConstraints);
}

_returnTemp = result;
Expand Down
19 changes: 18 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,24 @@ internal sealed override bool CallsAreOmitted(SyntaxTree syntaxTree)
return base.CallsAreOmitted(syntaxTree);
}

internal sealed override bool GenerateDebugInfo => !IsAsync && !IsIterator;
Copy link
Member Author

@333fred 333fred May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needed to change because the initial implementation method previously got replaced with a compiler-generated stub body, and we would never generate debug info. We now look at whether runtime async is enabled for this, but as I haven't added dedicated testing I am leaving a prototype to follow up.

internal sealed override bool GenerateDebugInfo
{
get
{
if (IsIterator)
{
return false;
}

if (IsAsync)
{
// PROTOTYPE: Need more dedicated debug information testing when runtime async is enabled.
return DeclaringCompilation.IsRuntimeAsyncEnabledIn(this);
}

return true;
}
}

#nullable enable
protected override void MethodChecks(BindingDiagnosticBag diagnostics)
Expand Down
215 changes: 214 additions & 1 deletion src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public class Exception
{
public Exception() {}
public Exception(string message) {}
public virtual string Message { get; }
}
public delegate TResult Func<TResult>();
public delegate TResult Func<T, TResult>(T arg);
Expand All @@ -107,7 +108,10 @@ public class NullReferenceException : Exception
{
public NullReferenceException(string message) : base(message) {}
}
public class Object {}
public class Object
{
public virtual string ToString() => null!;
}
public class String {}
public class Type {}
public class ValueType {}
Expand Down Expand Up @@ -8847,5 +8851,214 @@ public static void UnsafeAwaitAwaiter<TAwaiter>(TAwaiter awaiter) where TAwaiter
Diagnostic(ErrorCode.ERR_RefConstraintNotSatisfied, "default(System.Threading.Tasks.ValueTask<int>)").WithArguments("System.Runtime.CompilerServices.AsyncHelpers.Await<T>(System.Threading.Tasks.ValueTask<T>)", "T", "int").WithLocation(1, 7)
);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78529")]
public void ExceptionHandlerReturn()
{
var code = """
using System;
using System.Threading.Tasks;

System.Console.WriteLine(await C.Handler());

class C
{
public static async Task<int> Handler()
{
try
{
return await Throw(42);
}
catch (IntegerException ex)
{
return ex.Value;
}
}

#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously
public static async Task<int> Throw(int value) => throw new IntegerException(value);
}

public class IntegerException(int value) : Exception(value.ToString())
{
public int Value => value;
}
""";

var comp = CreateRuntimeAsyncCompilation(code);
var verifier = CompileAndVerify(comp, expectedOutput: ExpectedOutput("42", isRuntimeAsync: true), verify: Verification.Fails with
{
ILVerifyMessage = $$"""
{{ReturnValueMissing("<Main>$", "0xf")}}
[Handler]: Unexpected type on the stack. { Offset = 0x18, Found = Int32, Expected = ref 'System.Threading.Tasks.Task`1<int32>' }
"""
});
verifier.VerifyIL("C.Handler()", """
{
// Code size 25 (0x19)
.maxstack 1
.locals init (int V_0)
.try
{
IL_0000: ldc.i4.s 42
IL_0002: call "System.Threading.Tasks.Task<int> C.Throw(int)"
IL_0007: call "int System.Runtime.CompilerServices.AsyncHelpers.Await<int>(System.Threading.Tasks.Task<int>)"
IL_000c: stloc.0
IL_000d: leave.s IL_0017
}
catch IntegerException
{
IL_000f: callvirt "int IntegerException.Value.get"
IL_0014: stloc.0
IL_0015: leave.s IL_0017
}
IL_0017: ldloc.0
IL_0018: ret
}
""");

comp = CreateRuntimeAsyncCompilation(code, options: TestOptions.DebugExe.WithDebugPlusMode(true));
verifier = CompileAndVerify(comp, expectedOutput: ExpectedOutput("42", isRuntimeAsync: true), verify: Verification.Fails with
{
ILVerifyMessage = $$"""
{{ReturnValueMissing("<Main>$", "0x12")}}
[Handler]: Unexpected type on the stack. { Offset = 0x1f, Found = Int32, Expected = ref 'System.Threading.Tasks.Task`1<int32>' }
"""
});
verifier.VerifyIL("C.Handler()", """
{
// Code size 32 (0x20)
.maxstack 1
.locals init (int V_0,
int V_1,
IntegerException V_2) //ex
IL_0000: nop
.try
{
IL_0001: nop
IL_0002: ldc.i4.s 42
IL_0004: call "System.Threading.Tasks.Task<int> C.Throw(int)"
IL_0009: call "int System.Runtime.CompilerServices.AsyncHelpers.Await<int>(System.Threading.Tasks.Task<int>)"
IL_000e: stloc.0
IL_000f: ldloc.0
IL_0010: stloc.1
IL_0011: leave.s IL_001e
}
catch IntegerException
{
IL_0013: stloc.2
IL_0014: nop
IL_0015: ldloc.2
IL_0016: callvirt "int IntegerException.Value.get"
IL_001b: stloc.1
IL_001c: leave.s IL_001e
}
IL_001e: ldloc.1
IL_001f: ret
}
""");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78529")]
public void ExceptionHandlerReturn_NonAsyncMethod()
{
var code = """
using System;
using System.Threading.Tasks;

System.Console.WriteLine(await C.Handler());

class C
{
public static Task<int> Handler()
{
try
{
return Throw(42);
}
catch (IntegerException ex)
{
return Task.FromResult(ex.Value);
}
}

#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously
public static async Task<int> Throw(int value) => throw new IntegerException(value);
}

public class IntegerException(int value) : Exception(value.ToString())
{
public int Value => value;
}
""";

var comp = CreateRuntimeAsyncCompilation(code);
var verifier = CompileAndVerify(comp, expectedOutput: ExpectedOutput("42", isRuntimeAsync: true), verify: Verification.Fails with
{
ILVerifyMessage = $$"""
{{ReturnValueMissing("<Main>$", "0xf")}}
"""
});
verifier.VerifyIL("C.Handler()", """
{
// Code size 25 (0x19)
.maxstack 1
.locals init (System.Threading.Tasks.Task<int> V_0)
.try
{
IL_0000: ldc.i4.s 42
IL_0002: call "System.Threading.Tasks.Task<int> C.Throw(int)"
IL_0007: stloc.0
IL_0008: leave.s IL_0017
}
catch IntegerException
{
IL_000a: callvirt "int IntegerException.Value.get"
IL_000f: call "System.Threading.Tasks.Task<int> System.Threading.Tasks.Task.FromResult<int>(int)"
IL_0014: stloc.0
IL_0015: leave.s IL_0017
}
IL_0017: ldloc.0
IL_0018: ret
}
""");

comp = CreateRuntimeAsyncCompilation(code, options: TestOptions.DebugExe.WithDebugPlusMode(true));
verifier = CompileAndVerify(comp, expectedOutput: ExpectedOutput("42", isRuntimeAsync: true), verify: Verification.Fails with
{
ILVerifyMessage = $$"""
{{ReturnValueMissing("<Main>$", "0x12")}}
"""
});
verifier.VerifyIL("C.Handler()", """
{
// Code size 30 (0x1e)
.maxstack 1
.locals init (System.Threading.Tasks.Task<int> V_0,
Copy link
Member

@RikkiGibson RikkiGibson May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would probably be good to have a ValueTask test as well as some custom task-like test. The return temps in those cases also need to be unwrapped, right?

Copy link
Member Author

@333fred 333fred May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only ValueTask. Custom task-likes will go through the old codepath and get the full state machine.

If you don't mind, I'll add another test in the next PR.

IntegerException V_1) //ex
IL_0000: nop
.try
{
IL_0001: nop
IL_0002: ldc.i4.s 42
IL_0004: call "System.Threading.Tasks.Task<int> C.Throw(int)"
IL_0009: stloc.0
IL_000a: leave.s IL_001c
}
catch IntegerException
{
IL_000c: stloc.1
IL_000d: nop
IL_000e: ldloc.1
IL_000f: callvirt "int IntegerException.Value.get"
IL_0014: call "System.Threading.Tasks.Task<int> System.Threading.Tasks.Task.FromResult<int>(int)"
IL_0019: stloc.0
IL_001a: leave.s IL_001c
}
IL_001c: ldloc.0
IL_001d: ret
}
""");
}
}
}