Separate finished state

docs/features/async-streams.md

@@ -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                                     +--------------------------+
 ```
 

src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncIteratorMethodToStateMachineRewriter.cs

@@ -9,7 +9,6 @@
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.Emit;
 using Microsoft.CodeAnalysis.PooledObjects;
-using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp
 {
@@ -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;
 
@@ -77,6 +76,9 @@ internal AsyncIteratorMethodToStateMachineRewriter(
                 increasing: false);
         }
 
+        protected override StateMachineState FinishedState
+            => StateMachineState.IteratorFinishedState;
+
         protected override BoundStatement? GenerateMissingStateDispatch()
         {
             var asyncDispatch = base.GenerateMissingStateDispatch();
@@ -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));
         }
 

src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs

@@ -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)
             {
@@ -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);
@@ -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);
 

src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncRewriter.AsyncIteratorRewriter.cs

@@ -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),
@@ -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;
                 //  }
@@ -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)));
 
@@ -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;
                 //  }
@@ -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)));
 

src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs

@@ -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".
@@ -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);
@@ -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);

src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorRewriter.cs

@@ -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),

src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs

@@ -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;
@@ -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));
         }
 

src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.cs

@@ -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);

src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedNames.cs

@@ -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)

src/Compilers/CSharp/Test/Emit/BreakingChanges.cs

@@ -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
@@ -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
     {

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncEHTests.cs

@@ -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 };