Skip to content
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

Separate initial and finished states for enumerable and async-enumerables #76841

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Separate initial and finished states for enumerable and async-enumerables #76841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be4fa14
Add test baselines for final state
jcouv Dec 8, 2024
7746db4
Separate finished state
jcouv Dec 9, 2024
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
38 changes: 20 additions & 18 deletions docs/features/async-streams.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,41 +320,43 @@ In both cases, we will add a `if (disposeMode) /* jump to enclosing finally or e
#### State values and transitions

The enumerable starts with state -2.
Calling GetAsyncEnumerator sets the state to -3, or returns a fresh enumerator (also with state -3).
Calling GetAsyncEnumerator sets the state to -4, or returns a fresh enumerator (also with state -4).

From there, MoveNext will either:
- reach the end of the method (-2, we're done and disposed)
- reach the end of the method (-3, we're done and disposed)
- reach a `yield break` (state unchanged, dispose mode = true)
- reach a `yield return` (-N, decreasing from -4)
- reach a `yield return` (-N, decreasing from -5)
- reach an `await` (N, increasing from 0)

From suspended state N or -N, MoveNext will resume execution (-1).
But if the suspension was a `yield return` (-N), you could also call DisposeAsync, which resumes execution (-1) in dispose mode.

When in dispose mode, MoveNext continues to suspend (N) and resume (-1) until the end of the method is reached (-2).
When in dispose mode, MoveNext continues to suspend (N) and resume (-1) until the end of the method is reached (-3).

The result of invoking `DisposeAsync` from states -1 or N is unspecified. This compiler generates `throw new NotSupportException()` for those cases.

```
DisposeAsync await
+------------------------+ +------------------------> N
| | | |
v GetAsyncEnumerator | | resuming |
-2 --------------------> -3 --------> -1 <-------------------------+ Dispose mode = false
^ | | |
| done and disposed | | yield return |
+-----------------------------------+ +-----------------------> -N
await
+------------------------> N
| |
GetAsyncEnumerator | resuming |
-2 ------------------------- -4 ---> -1 <-------------------------+ Dispose mode = false
| | | |
DisposeAsync | | | yield return |
+-----------------------------+ | +-----------------------> -N
| | |
+-----------------------------------+ |
| done and disposed | |
| or exception thrown | |
| | |
| yield | |
| break | DisposeAsync |
| | +--------------------------+
| yield | DisposeAsync |
| break | +--------------------------+
| | |
| | |
| done and disposed v v suspension (await)
+----------------------------------- -1 ------------------------> N
^ | Dispose mode = true
| resuming |
+--------------------------+
| ^ | Dispose mode = true
v | resuming |
-3 +--------------------------+
```

13 changes: 5 additions & 8 deletions ...ilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncIteratorMethodToStateMachineRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.Emit;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp
{
Expand Down Expand Up @@ -40,7 +39,7 @@ internal sealed class AsyncIteratorMethodToStateMachineRewriter : AsyncMethodToS
private readonly LabelSymbol _exprReturnLabelTrue;

/// <summary>
/// States for `yield return` are decreasing from <see cref="StateMachineState.InitialAsyncIteratorState"/>.
/// States for `yield return` are decreasing from <see cref="StateMachineState.FirstResumableAsyncIteratorState"/>.
/// </summary>
private readonly ResumableStateMachineStateAllocator _iteratorStateAllocator;

Expand Down Expand Up @@ -77,6 +76,9 @@ internal AsyncIteratorMethodToStateMachineRewriter(
increasing: false);
}

protected override StateMachineState FinishedState
=> StateMachineState.IteratorFinishedState;

protected override BoundStatement? GenerateMissingStateDispatch()
{
var asyncDispatch = base.GenerateMissingStateDispatch();
Expand Down Expand Up @@ -222,12 +224,7 @@ private BoundStatement AppendJumpToCurrentDisposalLabel(BoundStatement node)

protected override BoundBinaryOperator ShouldEnterFinallyBlock()
{
// We should skip the finally block when:
// - the state is 0 or greater (we're suspending on an `await`)
// - the state is -3, -4 or lower (we're suspending on a `yield return`)
// We don't care about state = -2 (method already completed)

// So we only want to enter the finally when the state is -1
// We only want to enter the finally when the state is -1
return F.IntEqual(F.Local(cachedState), F.Literal(StateMachineState.NotStartedOrRunningState));
}

Expand Down
7 changes: 5 additions & 2 deletions src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ internal void GenerateMoveNext(BoundStatement body, MethodSymbol moveNextMethod)
bodyBuilder.Add(F.Label(_exprReturnLabel));

// this.state = finishedState
var stateDone = F.Assignment(F.Field(F.This(), stateField), F.Literal(StateMachineState.FinishedState));
var stateDone = F.Assignment(F.Field(F.This(), stateField), F.Literal(this.FinishedState));
var block = body.Syntax as BlockSyntax;
if (block == null)
{
Expand Down Expand Up @@ -225,7 +225,7 @@ BoundCatchBlock generateExceptionHandling(LocalSymbol exceptionLocal, ImmutableA

// _state = finishedState;
BoundStatement assignFinishedState =
F.ExpressionStatement(F.AssignmentExpression(F.Field(F.This(), stateField), F.Literal(StateMachineState.FinishedState)));
F.ExpressionStatement(F.AssignmentExpression(F.Field(F.This(), stateField), F.Literal(this.FinishedState)));

// builder.SetException(ex); OR if (this.combinedTokens != null) this.combinedTokens.Dispose(); _promiseOfValueOrEnd.SetException(ex);
BoundStatement callSetException = GenerateSetExceptionCall(exceptionLocal);
Expand All @@ -246,6 +246,9 @@ BoundCatchBlock generateExceptionHandling(LocalSymbol exceptionLocal, ImmutableA
}
}

protected virtual StateMachineState FinishedState
=> StateMachineState.AsyncFinishedState;

protected virtual BoundStatement GenerateTopLevelTry(BoundBlock tryBlock, ImmutableArray<BoundCatchBlock> catchBlocks)
=> F.Try(tryBlock, catchBlocks);

Expand Down
22 changes: 13 additions & 9 deletions src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncRewriter.AsyncIteratorRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ private BoundExpressionStatement GenerateCreateAndAssignBuilder()
protected override void InitializeStateMachine(ArrayBuilder<BoundStatement> bodyBuilder, NamedTypeSymbol frameType, LocalSymbol stateMachineLocal)
{
// var stateMachineLocal = new {StateMachineType}({initialState})
var initialState = _isEnumerable ? StateMachineState.FinishedState : StateMachineState.InitialAsyncIteratorState;
var initialState = _isEnumerable ? StateMachineState.InitialEnumerableState : StateMachineState.InitialAsyncIteratorState;
bodyBuilder.Add(
F.Assignment(
F.Local(stateMachineLocal),
Expand Down Expand Up @@ -279,8 +279,9 @@ protected override BoundStatement GenerateStateMachineCreation(LocalSymbol state
[SuppressMessage("Style", "VSTHRD200:Use \"Async\" suffix for async methods", Justification = "Standard naming convention for generating 'IAsyncEnumerator.MoveNextAsync'")]
private void GenerateIAsyncEnumeratorImplementation_MoveNextAsync()
{
Debug.Assert((int)StateMachineState.IteratorFinishedState == -3);
// Produce:
// if (state == StateMachineStates.FinishedStateMachine)
// if (state == StateMachineStates.IteratorFinishedState)
// {
// return default;
// }
Expand Down Expand Up @@ -327,8 +328,8 @@ private void GenerateIAsyncEnumeratorImplementation_MoveNextAsync()
out MethodSymbol promise_get_Version);

BoundStatement ifFinished = F.If(
// if (state == StateMachineStates.FinishedStateMachine)
F.IntEqual(F.InstanceField(stateField), F.Literal(StateMachineState.FinishedState)),
// if (state == StateMachineStates.IteratorFinishedState)
F.IntEqual(F.InstanceField(stateField), F.Literal(StateMachineState.IteratorFinishedState)),
// return default;
thenClause: F.Return(F.Default(moveNextAsyncReturnType)));

Expand Down Expand Up @@ -408,12 +409,15 @@ private void GetPartsForStartingMachine(out BoundExpressionStatement callReset,
[SuppressMessage("Style", "VSTHRD200:Use \"Async\" suffix for async methods", Justification = "Standard naming convention for generating 'IAsyncDisposable.DisposeAsync'")]
private void GenerateIAsyncDisposable_DisposeAsync()
{
Debug.Assert((int)StateMachineState.NotStartedOrRunningState == -1);
Debug.Assert((int)StateMachineState.IteratorFinishedState == -3);

// Produce:
// if (state >= StateMachineStates.NotStartedStateMachine /* -3 */)
// if (state >= StateMachineStates.NotStartedOrRunningState /* -1 */)
// {
// throw new NotSupportedException();
// }
// if (state == StateMachineStates.FinishedStateMachine /* -2 */)
// if (state == StateMachineStates.IteratorFinishedState /* -3 */)
// {
// return default;
// }
Expand All @@ -437,14 +441,14 @@ private void GenerateIAsyncDisposable_DisposeAsync()
out MethodSymbol promise_get_Version);

BoundStatement ifInvalidState = F.If(
// if (state >= StateMachineStates.NotStartedStateMachine /* -1 */)
// if (state >= StateMachineStates.NotStartedOrRunningState /* -1 */)
F.IntGreaterThanOrEqual(F.InstanceField(stateField), F.Literal(StateMachineState.NotStartedOrRunningState)),
// throw new NotSupportedException();
thenClause: F.Throw(F.New(F.WellKnownType(WellKnownType.System_NotSupportedException))));

BoundStatement ifFinished = F.If(
// if (state == StateMachineStates.FinishedStateMachine)
F.IntEqual(F.InstanceField(stateField), F.Literal(StateMachineState.FinishedState)),
// if (state == StateMachineStates.IteratorFinishedState /* -3 */)
F.IntEqual(F.InstanceField(stateField), F.Literal(StateMachineState.IteratorFinishedState)),
// return default;
thenClause: F.Return(F.Default(returnType)));

Expand Down
6 changes: 3 additions & 3 deletions ...mpilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ internal sealed partial class IteratorMethodToStateMachineRewriter : MethodToSta

/// <summary>
/// Finally state of the next Finally frame if such created.
/// Finally state is a negative decreasing number starting with -3. (-2 is used for something else).
/// Finally state is a negative decreasing number starting with -4 (<see cref="StateMachineState.FirstIteratorFinalizeState"/>). (-2 and -3 are used for something else).
/// Root frame has finally state -1.
///
/// The Finally state is the state that we are in when "between states".
Expand Down Expand Up @@ -163,7 +163,7 @@ internal void GenerateMoveNextAndDispose(BoundStatement body, SynthesizedImpleme
// nothing to finalize
var disposeBody = F.Block(
GenerateAllHoistedLocalsCleanup(),
F.Assignment(F.Field(F.This(), stateField), F.Literal(StateMachineState.FinishedState)),
F.Assignment(F.Field(F.This(), stateField), F.Literal(StateMachineState.IteratorFinishedState)),
F.Return());

F.CloseMethod(disposeBody);
Expand All @@ -178,7 +178,7 @@ internal void GenerateMoveNextAndDispose(BoundStatement body, SynthesizedImpleme
F.Assignment(F.Local(stateLocal), F.Field(F.This(), stateField)),
EmitFinallyFrame(rootFrame, state),
GenerateAllHoistedLocalsCleanup(),
F.Assignment(F.Field(F.This(), stateField), F.Literal(StateMachineState.FinishedState)),
F.Assignment(F.Field(F.This(), stateField), F.Literal(StateMachineState.IteratorFinishedState)),
F.Return());

F.CloseMethod(disposeBody);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ protected override void InitializeStateMachine(ArrayBuilder<BoundStatement> body
{
// var stateMachineLocal = new IteratorImplementationClass(N)
// where N is either 0 (if we're producing an enumerator) or -2 (if we're producing an enumerable)
var initialState = _isEnumerable ? StateMachineState.FinishedState : StateMachineState.InitialIteratorState;
var initialState = _isEnumerable ? StateMachineState.InitialEnumerableState : StateMachineState.InitialIteratorState;
bodyBuilder.Add(
F.Assignment(
F.Local(stateMachineLocal),
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,8 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node)
/// </summary>
public override BoundNode VisitTryStatement(BoundTryStatement node)
{
Debug.Assert(this is AsyncIteratorMethodToStateMachineRewriter or AsyncMethodToStateMachineRewriter);

var oldDispatches = _dispatches;

_dispatches = null;
Expand Down Expand Up @@ -926,6 +928,7 @@ protected virtual BoundBlock VisitFinally(BoundBlock finallyBlock)

protected virtual BoundBinaryOperator ShouldEnterFinallyBlock()
{
Debug.Assert(this is AsyncMethodToStateMachineRewriter);
return F.IntLessThan(F.Local(cachedState), F.Literal(StateMachineState.FirstUnusedState));
}

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ protected SynthesizedImplementationMethod GenerateIteratorGetEnumerator(MethodSy
makeIterator = F.If(
// if (this.state == -2 && this.initialThreadId == Thread.CurrentThread.ManagedThreadId)
condition: F.LogicalAnd(
F.IntEqual(F.Field(F.This(), stateField), F.Literal(StateMachineState.FinishedState)),
F.IntEqual(F.Field(F.This(), stateField), F.Literal(StateMachineState.InitialEnumerableState)),
F.IntEqual(F.Field(F.This(), initialThreadIdField), managedThreadId)),
thenClause: F.Block(thenBuilder.ToImmutableAndFree()),
elseClauseOpt: makeIterator);
Expand Down
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedNames.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@ internal static string MakePrimaryConstructorParameterFieldName(string parameter

internal static string MakeIteratorFinallyMethodName(StateMachineState finalizeState)
{
Debug.Assert((int)finalizeState < -2);
Debug.Assert((int)finalizeState <= (int)StateMachineState.FirstIteratorFinalizeState);
Debug.Assert((int)StateMachineState.FirstIteratorFinalizeState == -4);

// It is important that the name is only derived from the finalizeState, so that when
// editing method during EnC the Finally methods corresponding to matching states have matching names.
Debug.Assert((char)GeneratedNameKind.IteratorFinallyMethod == 'm');
return "<>m__Finally" + StringExtensions.GetNumeral(-((int)finalizeState + 2));
return "<>m__Finally" + StringExtensions.GetNumeral(-((int)finalizeState + 3));
}

internal static string MakeStaticLambdaDisplayClassName(int methodOrdinal, int generation)
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/Emit/BreakingChanges.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,7 @@ .locals init (bool V_0,
IL_0013: ldc.i4.m1
IL_0014: stfld ""int Program.test.<Goo>d__1.<>1__state""
IL_0019: ldarg.0
IL_001a: ldc.i4.s -3
IL_001a: ldc.i4.s -4
IL_001c: stfld ""int Program.test.<Goo>d__1.<>1__state""
IL_0021: ldarg.0
IL_0022: ldnull
Expand All @@ -1169,7 +1169,7 @@ .locals init (bool V_0,
IL_0030: stloc.0
IL_0031: leave.s IL_0063
IL_0033: ldarg.0
IL_0034: ldc.i4.s -3
IL_0034: ldc.i4.s -4
IL_0036: stfld ""int Program.test.<Goo>d__1.<>1__state""
.try
{
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncEHTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

namespace Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen
{
[CompilerTrait(CompilerFeature.Async)]
public class CodeGenAsyncEHTests : EmitMetadataTestBase
{
private static readonly MetadataReference[] s_asyncRefs = new[] { MscorlibRef_v4_0_30316_17626, SystemRef_v4_0_30319_17929, SystemCoreRef_v4_0_30319_17929 };
Expand Down
Loading
Loading