From c58cbe164886d5e32de3285a50c2e903dea1e67d Mon Sep 17 00:00:00 2001 From: tmat <tomas.matousek@microsoft.com> Date: Fri, 27 May 2022 16:08:12 -0700 Subject: [PATCH] Feedback --- ...yncIteratorMethodToStateMachineRewriter.cs | 6 +-- .../AsyncMethodToStateMachineRewriter.cs | 6 +-- .../AsyncRewriter.AsyncIteratorRewriter.cs | 8 ++-- .../Lowering/AsyncRewriter/AsyncRewriter.cs | 2 +- ...ateMachineRewriter.IteratorFinallyFrame.cs | 2 +- .../IteratorMethodToStateMachineRewriter.cs | 2 +- .../IteratorRewriter/IteratorRewriter.cs | 2 +- .../StateMachineRewriter.cs | 2 +- .../StateMachineStates.cs | 6 +-- .../Portable/CodeGen/StateMachineStates.cs | 41 ------------------- .../Portable/CodeGen/VariableSlotAllocator.cs | 2 +- 11 files changed, 19 insertions(+), 60 deletions(-) delete mode 100644 src/Compilers/Core/Portable/CodeGen/StateMachineStates.cs diff --git a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncIteratorMethodToStateMachineRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncIteratorMethodToStateMachineRewriter.cs index 77800ee3cce04..6a5506952a903 100644 --- a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncIteratorMethodToStateMachineRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncIteratorMethodToStateMachineRewriter.cs @@ -217,7 +217,7 @@ protected override BoundBinaryOperator ShouldEnterFinallyBlock() // We don't care about state = -2 (method already completed) // So we only want to enter the finally when the state is -1 - return F.IntEqual(F.Local(cachedState), F.Literal(StateMachineStates.NotStartedStateMachine)); + return F.IntEqual(F.Local(cachedState), F.Literal(StateMachineStates.NotStartedOrRunningState)); } #region Visitors @@ -244,7 +244,7 @@ protected override BoundStatement VisitBody(BoundStatement body) return F.Block( F.Label(resumeLabel), // initialStateResumeLabel: GenerateJumpToCurrentDisposalLabel(), // if (disposeMode) goto _exprReturnLabel; - GenerateSetBothStates(StateMachineStates.NotStartedStateMachine), // this.state = cachedState = -1; + GenerateSetBothStates(StateMachineStates.NotStartedOrRunningState), // this.state = cachedState = -1; rewrittenBody); } @@ -288,7 +288,7 @@ public override BoundNode VisitYieldReturnStatement(BoundYieldReturnStatement no blockBuilder.Add( // this.state = cachedState = NotStartedStateMachine - GenerateSetBothStates(StateMachineStates.NotStartedStateMachine)); + GenerateSetBothStates(StateMachineStates.NotStartedOrRunningState)); Debug.Assert(_currentDisposalLabel is object); // no yield return allowed inside a finally blockBuilder.Add( diff --git a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs index e0ebe2f938bf9..65634991addeb 100644 --- a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs @@ -161,7 +161,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(StateMachineStates.FinishedStateMachine)); + var stateDone = F.Assignment(F.Field(F.This(), stateField), F.Literal(StateMachineStates.FinishedState)); var block = body.Syntax as BlockSyntax; if (block == null) { @@ -235,7 +235,7 @@ protected BoundCatchBlock GenerateExceptionHandling(LocalSymbol exceptionLocal, // _state = finishedState; BoundStatement assignFinishedState = - F.ExpressionStatement(F.AssignmentExpression(F.Field(F.This(), stateField), F.Literal(StateMachineStates.FinishedStateMachine))); + F.ExpressionStatement(F.AssignmentExpression(F.Field(F.This(), stateField), F.Literal(StateMachineStates.FinishedState))); // builder.SetException(ex); OR if (this.combinedTokens != null) this.combinedTokens.Dispose(); _promiseOfValueOrEnd.SetException(ex); BoundStatement callSetException = GenerateSetExceptionCall(exceptionLocal); @@ -497,7 +497,7 @@ private BoundBlock GenerateAwaitForIncompleteTask(LocalSymbol awaiterTemp) blockBuilder.Add( // this.state = cachedState = NotStartedStateMachine - GenerateSetBothStates(StateMachineStates.NotStartedStateMachine)); + GenerateSetBothStates(StateMachineStates.NotStartedOrRunningState)); return F.Block(blockBuilder.ToImmutableAndFree()); } diff --git a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncRewriter.AsyncIteratorRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncRewriter.AsyncIteratorRewriter.cs index 4e1f1c4ae966b..d65f85549dbca 100644 --- a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncRewriter.AsyncIteratorRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncRewriter.AsyncIteratorRewriter.cs @@ -195,7 +195,7 @@ private BoundExpressionStatement GenerateCreateAndAssignBuilder() protected override void InitializeStateMachine(ArrayBuilder<BoundStatement> bodyBuilder, NamedTypeSymbol frameType, LocalSymbol stateMachineLocal) { // var stateMachineLocal = new {StateMachineType}({initialState}) - int initialState = _isEnumerable ? StateMachineStates.FinishedStateMachine : StateMachineStates.InitialAsyncIteratorState; + int initialState = _isEnumerable ? StateMachineStates.FinishedState : StateMachineStates.InitialAsyncIteratorState; bodyBuilder.Add( F.Assignment( F.Local(stateMachineLocal), @@ -323,7 +323,7 @@ private void GenerateIAsyncEnumeratorImplementation_MoveNextAsync() BoundStatement ifFinished = F.If( // if (state == StateMachineStates.FinishedStateMachine) - F.IntEqual(F.InstanceField(stateField), F.Literal(StateMachineStates.FinishedStateMachine)), + F.IntEqual(F.InstanceField(stateField), F.Literal(StateMachineStates.FinishedState)), // return default; thenClause: F.Return(F.Default(moveNextAsyncReturnType))); @@ -433,13 +433,13 @@ private void GenerateIAsyncDisposable_DisposeAsync() BoundStatement ifInvalidState = F.If( // if (state >= StateMachineStates.NotStartedStateMachine /* -1 */) - F.IntGreaterThanOrEqual(F.InstanceField(stateField), F.Literal(StateMachineStates.NotStartedStateMachine)), + F.IntGreaterThanOrEqual(F.InstanceField(stateField), F.Literal(StateMachineStates.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(StateMachineStates.FinishedStateMachine)), + F.IntEqual(F.InstanceField(stateField), F.Literal(StateMachineStates.FinishedState)), // return default; thenClause: F.Return(F.Default(returnType))); diff --git a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncRewriter.cs index 200ce0936ff43..6616076c64846 100644 --- a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncRewriter.cs @@ -230,7 +230,7 @@ protected override BoundStatement GenerateStateMachineCreation(LocalSymbol state bodyBuilder.Add( F.Assignment( F.Field(F.Local(stateMachineVariable), stateField.AsMember(frameType)), - F.Literal(StateMachineStates.NotStartedStateMachine))); + F.Literal(StateMachineStates.NotStartedOrRunningState))); // local.$builder.Start(ref local) -- binding to the method AsyncTaskMethodBuilder<typeArgs>.Start() var startMethod = methodScopeAsyncMethodBuilderMemberCollection.Start.Construct(frameType); diff --git a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.IteratorFinallyFrame.cs b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.IteratorFinallyFrame.cs index d1acb500db5af..24656238528aa 100644 --- a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.IteratorFinallyFrame.cs +++ b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.IteratorFinallyFrame.cs @@ -57,7 +57,7 @@ public IteratorFinallyFrame( public IteratorFinallyFrame() { - this.finalizeState = StateMachineStates.NotStartedStateMachine; + this.finalizeState = StateMachineStates.NotStartedOrRunningState; } public bool IsRoot() diff --git a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs index 4e4012ea696c0..2b647596d65c3 100644 --- a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs @@ -118,7 +118,7 @@ internal void GenerateMoveNextAndDispose(BoundStatement body, SynthesizedImpleme Dispatch(isOutermost: true), GenerateReturn(finished: true), F.Label(initialLabel), - F.Assignment(F.Field(F.This(), stateField), F.Literal(StateMachineStates.NotStartedStateMachine)), + F.Assignment(F.Field(F.This(), stateField), F.Literal(StateMachineStates.NotStartedOrRunningState)), newBody); // diff --git a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorRewriter.cs index 991dc3ca4ea6e..a1429a11033e7 100644 --- a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorRewriter.cs @@ -287,7 +287,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) - int initialState = _isEnumerable ? StateMachineStates.FinishedStateMachine : StateMachineStates.InitialIteratorState; + int initialState = _isEnumerable ? StateMachineStates.FinishedState : StateMachineStates.InitialIteratorState; bodyBuilder.Add( F.Assignment( F.Local(stateMachineLocal), diff --git a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.cs index ae973623c1dc2..0ef6f322a3bd2 100644 --- a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.cs @@ -437,7 +437,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(StateMachineStates.FinishedStateMachine)), + F.IntEqual(F.Field(F.This(), stateField), F.Literal(StateMachineStates.FinishedState)), F.IntEqual(F.Field(F.This(), initialThreadIdField), managedThreadId)), thenClause: F.Block(thenBuilder.ToImmutableAndFree()), elseClauseOpt: makeIterator); diff --git a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineStates.cs b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineStates.cs index 92dcf9405415c..3d1b0a70a1fc2 100644 --- a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineStates.cs +++ b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineStates.cs @@ -12,8 +12,8 @@ internal static class StateMachineStates { internal const int FirstIteratorFinalizeState = -3; - internal const int FinishedStateMachine = -2; - internal const int NotStartedStateMachine = -1; + internal const int FinishedState = -2; + internal const int NotStartedOrRunningState = -1; internal const int FirstUnusedState = 0; /// <summary> @@ -30,7 +30,7 @@ internal static class StateMachineStates internal const int FirstResumableIteratorState = InitialIteratorState + 1; /// <summary> - /// Used for async-iterators to distinguish initial state from running state (-1) so that DisposeAsync can throw in latter case. + /// Used for async-iterators to distinguish initial state from <see cref="NotStartedOrRunningState"/> so that DisposeAsync can throw in latter case. /// </summary> internal const int InitialAsyncIteratorState = -3; diff --git a/src/Compilers/Core/Portable/CodeGen/StateMachineStates.cs b/src/Compilers/Core/Portable/CodeGen/StateMachineStates.cs deleted file mode 100644 index 895d464fdbd0a..0000000000000 --- a/src/Compilers/Core/Portable/CodeGen/StateMachineStates.cs +++ /dev/null @@ -1,41 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using System.Diagnostics; - -namespace Microsoft.CodeAnalysis.CodeGen; - -internal static class StateMachineStates -{ - internal const int FirstIteratorFinalizeState = -3; - - internal const int FinishedStateMachine = -2; - internal const int NotStartedStateMachine = -1; - internal const int FirstUnusedState = 0; - - /// <summary> - /// First state in async state machine that is used to resume the machine after await. - /// </summary> - internal const int FirstResumableAsyncState = 0; - - internal const int InitialIteratorState = 0; - - /// <summary> - /// First state in iterator state machine that is used to resume the machine after yield return. - /// Initial state is not used to resume state machine that yielded. - /// </summary> - internal const int FirstResumableIteratorState = InitialIteratorState + 1; - - /// <summary> - /// Used for async-iterators to distinguish initial state from running state (-1) so that DisposeAsync can throw in latter case. - /// </summary> - internal const int InitialAsyncIteratorState = -3; - - /// <summary> - /// First state in async iterator state machine that is used to resume the machine after yield return. - /// Initial state is not used to resume state machine that yielded. - /// </summary> - internal const int FirstResumableAsyncIteratorState = InitialAsyncIteratorState - 1; -} diff --git a/src/Compilers/Core/Portable/CodeGen/VariableSlotAllocator.cs b/src/Compilers/Core/Portable/CodeGen/VariableSlotAllocator.cs index 8219dc698af74..d0bac38235b1f 100644 --- a/src/Compilers/Core/Portable/CodeGen/VariableSlotAllocator.cs +++ b/src/Compilers/Core/Portable/CodeGen/VariableSlotAllocator.cs @@ -89,7 +89,7 @@ public abstract bool TryGetPreviousHoistedLocalSlotIndex( public abstract int? GetFirstUnusedStateMachineState(bool increasing); /// <summary> - /// For a given node associated with a entering a state of a state machine in the new compilation, + /// For a given node associated with entering a state of a state machine in the new compilation, /// returns the ordinal of the corresponding state in the previous version of the state machine. /// </summary> /// <returns>