Fix async hoisted type substituted local#78231
Merged
AlekseyTs merged 23 commits intodotnet:mainfrom Apr 29, 2025
Merged
Conversation
…istRefInitialization
AlekseyTs
reviewed
Apr 22, 2025
src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs
Outdated
Show resolved
Hide resolved
AlekseyTs
reviewed
Apr 22, 2025
src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs
Outdated
Show resolved
Hide resolved
AlekseyTs
reviewed
Apr 22, 2025
src/Compilers/CSharp/Test/Semantic/Semantics/AwaitExpressionTests.cs
Outdated
Show resolved
Hide resolved
AlekseyTs
reviewed
Apr 22, 2025
src/Compilers/CSharp/Test/Semantic/Semantics/AwaitExpressionTests.cs
Outdated
Show resolved
Hide resolved
Contributor
|
Done with review pass (commit 2) |
Contributor
Author
|
I found a new issue which I have fixed in this PR, too. using System.Threading.Tasks;
public sealed class RefHolder<T>
{
private T _t;
public ref T Get() => ref _t;
}
public static class App
{
extension(int)
{
public static void Do<T>()
{
var res = new RefHolder<T>();
M().Wait();
async Task M()
{
res.Get() = await Task.FromResult(default(T));
}
}
}
}The compiler crashed with: |
Open
3 tasks
AlekseyTs
reviewed
Apr 23, 2025
src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs
Outdated
Show resolved
Hide resolved
AlekseyTs
reviewed
Apr 23, 2025
AlekseyTs
reviewed
Apr 23, 2025
src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs
Outdated
Show resolved
Hide resolved
Contributor
|
Done with review pass (commit 21) |
Contributor
|
Done with review pass (commit 22) |
Contributor
|
@dotnet/roslyn-compiler For the second review |
Contributor
Author
|
@jaredpar can you have a Look? |
Contributor
|
@dotnet/roslyn-compiler For the second review for a community PR |
jaredpar
approved these changes
Apr 28, 2025
Contributor
|
@bernd5 Thanks for the contribution! |
This was referenced May 1, 2025
333fred
added a commit
to 333fred/roslyn
that referenced
this pull request
May 8, 2025
333fred
added a commit
that referenced
this pull request
Aug 19, 2025
* Handle basic await scenarios (#76121) Add initial handling of expressions that return `Task`, `Task<T>`, `ValueTask`, `ValueTask<T>`. * Add RuntimeAsyncMethodGenerationAttribute (#77543) Adds control for whether to use runtime async. The flowchart is as follows: 1. The flag `System.Runtime.CompilerServices.RuntimeFeature.Async` must be present. 2. Assuming that flag is present, we look for the presence of `System.Runtime.CompilerServices.RuntimeAsyncMethodGenerationAttribute` on the method. If that attribute is present, we use the preference expressed in the attribute. The preference does not carry to nested contexts, such as local functions or lambdas. 3. If the attribute is not present, we look for `features:runtime-async=on` on the command line. If that is present, then the feature is on by default. Otherwise, the feature is off. * Semantic search info * Implement custom awaitable support (#78071) This adds support for awaiting task-like types that are not natively supported by runtime async. Closes #77897. * Move runtime async method validation into initial binding (#78310) We now do method construction and validation for runtime async helpers up front in initial binding, rather than doing it in `RuntimeAsyncRewriter`. I've also renamed the APIs as per dotnet/runtime#114310 (comment) (though I haven't added ConfigureAwait support yet, that will be the next PR). We now validate: * The helpers come from `System.Runtime.CompilerServices.AsyncHelpers`, defined in corelib. This means that I now need a fairly extensive corelib mock to be able to compile. When we have a testing runtime that defines these helpers, we can remove the giant mock and use the real one. * We properly error when expected helpers aren't present. * We properly check to make sure that constraints are satisfied when doing generic substitution in one of the runtime helpers. * Runtime async is not turned on if the async method does not return `Task`, `Task<T>`, `ValueTask`, or `ValueTask<T>`. Relates to test plan #75960 * React to main changes #78246 and #78231. * Switch MethodImplAttributes.Async to 0x2000 (#78536) It was changed in dotnet/runtime#114310 as 0x400 is a thing in framework. * Ensure return local is the correct type for runtime async (#78603) * Add test demonstrating current behavior * Ensure return local is the correct type in async scenarios * Ensure method is actually async when doing local rewrite * Exception Handler support (#78773) * Update EH tests to run with runtime async * Handle non-null exception filter prologues in the spill sequencer * Add more testing to show current incorrect behavior * Unskip ConditionalFacts that do not need to be skipped. * Handle ensuring that the method remains valid, even when there is an `await` in a finally section. Add signifcant testing of `await using`. * Fix baselines * Support `await foreach` and add runtime async verification to existing tests. * Remove unnecessary generic * Failing tests, add async void test suggestion * CI failures * Add additional test * Test fixes * Remove implemented PROTOTYPE, add assertion on behavior. * Update to SpillSequenceSpiller after some more debugging and tightening the assertion * Fix nullref * Enable nullable for VisitCatchBlock * Support using a simple overload resolution for finding Await helpers from the BCL (#79135) * Support using a simple overload resolution for finding Await helpers from the BCL This PR removes special knowledge of what `Await` helpers correspond to what types, and instead implements a very simple form of overload resolution. We immediately bail on any conflict or error and fall back to attempting to use `AwaitAwaiter` or `UnsafeAwaitAwaiter` when such scenarios are detected. I've also updated the rules to better reflect what is actually implementable. * Create the full BoundCall in initial binding. * PR feedback. * Baseline struct lifting tests (#79505) * Extract expectedOutput constants, minor touchups * Rename expected -> expectedOutput * Include new testing with placeholder baselines * Progress * First ILVerify pass * Initial baseline IL run. * Further baseline additions and skips based on missing corelib apis. * Clone async void tests and have them use async Task, and validate existing code spit for these under runtime async * Update baselines after .NET 10 intake * Delete the stub * Remove long dynamic baseline and leave a prototype. * Feedback. * BOM * Remove unused references parameter * Block `await dynamic` * Block hoisted variables from runtime async for now * Update test baselines for block * Block arglist in runtime async * Add IL baseline * Handle an additional branch beyond the end of the method case. * Move prototype comments to issues. * Remove entry point prototypes * Add assert and comment * Add back assert * Report obsolete/experimental diagnostics on await helpers. * Fix ref safety analysis build error. --------- Signed-off-by: Emmanuel Ferdman <emmanuelferdman@gmail.com> Co-authored-by: Jan Jones <janjones@microsoft.com> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Cyrus Najmabadi <cyrusn@microsoft.com> Co-authored-by: David Barbet <dabarbet@microsoft.com> Co-authored-by: Ankita Khera <40616383+akhera99@users.noreply.github.com> Co-authored-by: David Wengier <david.wengier@microsoft.com> Co-authored-by: Rikki Gibson <rigibson@microsoft.com> Co-authored-by: Cyrus Najmabadi <cyrus.najmabadi@gmail.com> Co-authored-by: Rikki Gibson <rikkigibson@gmail.com> Co-authored-by: akhera99 <ankitakhera@microsoft.com> Co-authored-by: Joey Robichaud <joseph.robichaud@microsoft.com> Co-authored-by: Todd Grunke <toddgrun@microsoft.com> Co-authored-by: Youssef1313 <youssefvictor00@gmail.com> Co-authored-by: Joey Robichaud <jorobich@microsoft.com> Co-authored-by: Tomáš Matoušek <tmat@users.noreply.github.com> Co-authored-by: Amadeusz Wieczorek <amwieczo@microsoft.com> Co-authored-by: Charles Stoner <10732005+cston@users.noreply.github.com> Co-authored-by: Jared Parsons <jared@paranoidcoding.org> Co-authored-by: Sam Harwell <Sam.Harwell@microsoft.com> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Jason Malinowski <jason.malinowski@microsoft.com> Co-authored-by: Etienne Baudoux <etbaudou@microsoft.com> Co-authored-by: AlekseyTs <AlekseyTs@users.noreply.github.com> Co-authored-by: Jan Jones <jan.jones.cz@gmail.com> Co-authored-by: Maryam Ariyan <maariyan@microsoft.com> Co-authored-by: Andrew Hall <andrha@microsoft.com> Co-authored-by: Arun Chander <arkalyan@microsoft.com> Co-authored-by: Kauwai Lucchesi <53876126+kauwai@users.noreply.github.com> Co-authored-by: Bill Wagner <wiwagn@microsoft.com> Co-authored-by: PaddiM8 <hi@bakk.dev> Co-authored-by: Matteo Prosperi <maprospe@microsoft.com> Co-authored-by: Julien Couvreur <julien.couvreur@gmail.com> Co-authored-by: Matteo Prosperi <41970398+matteo-prosperi@users.noreply.github.com> Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com> Co-authored-by: Tomas Matousek <tomat@microsoft.com> Co-authored-by: Deepak Rathore (ALLYIS INC) <v-deerathore@microsoft.com> Co-authored-by: Emmanuel Ferdman <emmanuelferdman@gmail.com> Co-authored-by: Evgeny Tvorun <evgenyt@microsoft.com> Co-authored-by: Victor Pogor <victor.pogor@outlook.com> Co-authored-by: Ella Hathaway <67609881+ellahathaway@users.noreply.github.com> Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com> Co-authored-by: Jason Malinowski <jason@jason-m.com> Co-authored-by: DoctorKrolic <mapmyp03@gmail.com> Co-authored-by: John Douglas Leitch <JohnLeitch@outlook.com> Co-authored-by: Matt Thalman <mthalman@microsoft.com> Co-authored-by: Bernd Baumanns <baumannsbernd@gmail.com> Co-authored-by: Thomas Shephard <thomas@thomas-shephard.com> Co-authored-by: DoctorKrolic <70431552+DoctorKrolic@users.noreply.github.com> Co-authored-by: David Barbet <dibarbet@gmail.com> Co-authored-by: Chris Sienkiewicz <chsienki@microsoft.com> Co-authored-by: tmat <tomas.matousek@microsoft.com> Co-authored-by: gel@microsoft.com <gel@microsoft.com> Co-authored-by: Gen Lu <genlu@users.noreply.github.com> Co-authored-by: Oleg Tkachenko <olegtk@microsoft.com> Co-authored-by: Matt Mitchell <mmitche@microsoft.com> Co-authored-by: Djuradj Kurepa <91743470+dkurepa@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Stuart Lang <stuart.b.lang@gmail.com> Co-authored-by: RaymondY <yangrongwei@hotmail.com> Co-authored-by: Gobind Singh <gobindsingh2004@gmail.com> Co-authored-by: David Kean <davkean@microsoft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #76999
Support TypeSubstitutedLocalSymbol's in MethodToStateMachineRewriter.HoistRefInitialization