Skip to content

Fix error when hoisting a non-ref call#80138

Merged
jjonescz merged 3 commits intodotnet:mainfrom
jjonescz:79415-AwaitBoundary
Sep 11, 2025
Merged

Fix error when hoisting a non-ref call#80138
jjonescz merged 3 commits intodotnet:mainfrom
jjonescz:79415-AwaitBoundary

Conversation

@jjonescz
Copy link
Member

@jjonescz jjonescz commented Sep 4, 2025

Fixes #79415.

@jjonescz jjonescz marked this pull request as ready for review September 4, 2025 16:00
@jjonescz jjonescz requested a review from a team as a code owner September 4, 2025 16:00
@jjonescz jjonescz requested a review from AlekseyTs September 4, 2025 16:00
}
}
""";
CompileAndVerify(source, expectedOutput: "123123").VerifyDiagnostics();
Copy link
Member

Choose a reason for hiding this comment

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

Please verify runtime async for this test as well, see examples in this file. You're going to conflict with #80093, and depending on who gets in first, one of us is going to need to adjust. I expect there to be an error for this test with runtime async right now; #79763 tracks this, please leave a comment here too.

Please consider moving this test to CodeGenAsyncSpillTests.cs, since that's where the rest of the spilling tests are.

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Sep 5, 2025

    var verifier = CompileAndVerify(comp, expectedOutput: "123123123").VerifyDiagnostics();

Consider adjusting the expected output as well #Closed


Refers to: src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests2.cs:15267 in 623ae77. [](commit_id = 623ae77, deletion_comment = False)


static async Task Test3<T>() where T : I1
{
GetT<T>()[0] += await Get1Async();
Copy link
Contributor

@AlekseyTs AlekseyTs Sep 5, 2025

Choose a reason for hiding this comment

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

GetT()[0] += await Get1Async();

Consider strengthening the test and observing order of evaluation in this statement. For example, replacing 0 with a function call and printing something distinct from each function and the accessors involved. With the current output we cannot even tell accessors apart. #Closed

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Sep 5, 2025

Done with review pass (commit 1) #Closed

await Test3<S1>();
}

static T GetT<T>() where T : I1 => (T)(object)new S1 { F1 = 123 };
Copy link
Contributor

@AlekseyTs AlekseyTs Sep 5, 2025

Choose a reason for hiding this comment

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

static T GetT() where T : I1 => (T)(object)new S1 { F1 = 123 };

It looks like we don't observe when and how many times this method is called. #Closed


var comp = CreateRuntimeAsyncCompilation(source);
comp.VerifyEmitDiagnostics(
// (38,31): error CS9328: Method 'Program.Test3<T>()' uses a feature that is not supported by runtime async currently. Opt the method out of runtime async by attributing it with 'System.Runtime.CompilerServices.RuntimeAsyncMethodGenerationAttribute(false)'.
Copy link
Contributor

@AlekseyTs AlekseyTs Sep 5, 2025

Choose a reason for hiding this comment

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

// (38,31): error CS9328: Method 'Program.Test3()' uses a feature that is not supported by runtime async currently. Opt the method out of runtime async by attributing it with

Is the error expected? What exactly isn't supported? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Hoisting isn't supported, it's being added in #80093.

@AlekseyTs
Copy link
Contributor

Done with review pass (commit 2)

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 3)

@jjonescz jjonescz requested a review from 333fred September 5, 2025 16:35
@jjonescz
Copy link
Member Author

@333fred for another look, thanks

@jjonescz jjonescz merged commit b83e1a5 into dotnet:main Sep 11, 2025
24 checks passed
@jjonescz jjonescz deleted the 79415-AwaitBoundary branch September 11, 2025 17:03
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Sep 11, 2025
333fred added a commit to 333fred/roslyn that referenced this pull request Sep 11, 2025
* upstream/main: (233 commits)
  Extensions: add SyntaxGenerator support and AssociatedExtensionImplementation API (dotnet#80170)
  Fix error when hoisting a non-ref call (dotnet#80138)
  Ensure that refkinds are rewritten for complex methods (dotnet#79916)
  Revert
  Do not go through the workspace to access services
  DefiniteAssignmentPass.MarkFieldsUsed - avoid infinite recursion due to generic substitution (dotnet#80135)
  Reduce allocations in AnalyzerDriver.TryExecuteSymbolEndActions (dotnet#79855)
  RefSafetyAnalysis: Fix handling of nested deconstruction utilizing modern extensions (dotnet#80231)
  Extensions: adjust rewriting of anonymous type property symbols (dotnet#80211)
  Extensions: Move public APIs into INamedTypeSymbol (dotnet#80230)
  Extensions: improve error recovery in older language versions (dotnet#80206)
  Fall back to `dotnet exec` if apphost does not exist (dotnet#80153)
  Update dependencies from https://github.com/dotnet/dotnet build 282708 (dotnet#80228)
  Add a workaround for microsoft/vs-mef#620
  Revert "FailFast if the MEF composition is clearly broken"
  switch from windows combobox to visualstudio combobox (dotnet#80219)
  Update System.Text.Json in packages which use 4.12 Roslyn (dotnet#80197)
  add flags to unblock CI (dotnet#80222)
  Move static members to another type - qualifies static member references in the moved members (dotnet#80178)
  Fix broken link for C# 14 lambda parameter modifiers
  ...
333fred added a commit to 333fred/roslyn that referenced this pull request Sep 11, 2025
@akhera99 akhera99 modified the milestones: Next, 18.0 P1, 18.0 P2 Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An unexpected error during async rewrite of generic compound assignment to a member of an RValue

4 participants