From 59a05a571c455e0a9cc3b7b6b63529e1da2d297a Mon Sep 17 00:00:00 2001 From: tmat Date: Thu, 19 May 2022 10:05:25 -0700 Subject: [PATCH] Remove unnecessary finalizer state handling from MethodToStateMachineRewriter --- .../AsyncMethodToStateMachineRewriter.cs | 2 +- .../IteratorMethodToStateMachineRewriter.cs | 2 +- .../MethodToStateMachineRewriter.cs | 73 ++----------------- 3 files changed, 8 insertions(+), 69 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs index de5d554e25fa2..54d51eeb76004 100644 --- a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs @@ -77,7 +77,7 @@ internal AsyncMethodToStateMachineRewriter( VariableSlotAllocator slotAllocatorOpt, int nextFreeHoistedLocalSlot, BindingDiagnosticBag diagnostics) - : base(F, method, state, hoistedVariables, nonReusableLocalProxies, synthesizedLocalOrdinals, slotAllocatorOpt, nextFreeHoistedLocalSlot, diagnostics, useFinalizerBookkeeping: false) + : base(F, method, state, hoistedVariables, nonReusableLocalProxies, synthesizedLocalOrdinals, slotAllocatorOpt, nextFreeHoistedLocalSlot, diagnostics) { _method = method; _asyncMethodBuilderMemberCollection = asyncMethodBuilderMemberCollection; diff --git a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs index e77349419b340..8de0bbf1aba92 100644 --- a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs @@ -66,7 +66,7 @@ internal IteratorMethodToStateMachineRewriter( VariableSlotAllocator slotAllocatorOpt, int nextFreeHoistedLocalSlot, BindingDiagnosticBag diagnostics) - : base(F, originalMethod, state, hoistedVariables, nonReusableLocalProxies, synthesizedLocalOrdinals, slotAllocatorOpt, nextFreeHoistedLocalSlot, diagnostics, useFinalizerBookkeeping: false) + : base(F, originalMethod, state, hoistedVariables, nonReusableLocalProxies, synthesizedLocalOrdinals, slotAllocatorOpt, nextFreeHoistedLocalSlot, diagnostics) { _current = current; } diff --git a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs index 134eae83f17d5..b0fb061e344da 100644 --- a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs @@ -22,13 +22,6 @@ internal abstract class MethodToStateMachineRewriter : MethodToClassRewriter { internal readonly MethodSymbol OriginalMethod; - /// - /// True if we need to generate the code to do the bookkeeping so we can "finalize" the state machine - /// by executing code from its current state through the enclosing finally blocks. This is true for - /// iterators and false for async. - /// - private readonly bool _useFinalizerBookkeeping; - /// /// Generate return statements from the state machine method body. /// @@ -72,21 +65,6 @@ internal abstract class MethodToStateMachineRewriter : MethodToClassRewriter /// private Dictionary> _dispatches = new Dictionary>(); - /// - /// A try block might have no state (transitions) within it, in which case it does not need - /// to have a state to represent finalization. This flag tells us whether the current try - /// block that we are within has a finalizer state. Initially true as we have the (trivial) - /// finalizer state of -1 at the top level. Not used if !this.useFinalizerBookkeeping. - /// - private bool _hasFinalizerState = true; - - /// - /// If hasFinalizerState is true, this is the state for finalization from anywhere in this - /// try block. Initially set to -1, representing the no-op finalization required at the top - /// level. Not used if !this.useFinalizerBookkeeping. - /// - private int _currentFinalizerState = -1; - /// /// A pool of fields used to hoist locals. They appear in this set when not in scope, /// so that members of this set may be allocated to locals when the locals come into scope. @@ -122,8 +100,7 @@ public MethodToStateMachineRewriter( SynthesizedLocalOrdinalsDispenser synthesizedLocalOrdinals, VariableSlotAllocator slotAllocatorOpt, int nextFreeHoistedLocalSlot, - BindingDiagnosticBag diagnostics, - bool useFinalizerBookkeeping) + BindingDiagnosticBag diagnostics) : base(slotAllocatorOpt, F.CompilationState, diagnostics) { Debug.Assert(F != null); @@ -137,8 +114,6 @@ public MethodToStateMachineRewriter( this.F = F; this.stateField = state; this.cachedState = F.SynthesizedLocal(F.SpecialType(SpecialType.System_Int32), syntax: F.Syntax, kind: SynthesizedLocalKind.StateMachineCachedState); - _useFinalizerBookkeeping = useFinalizerBookkeeping; - _hasFinalizerState = useFinalizerBookkeeping; this.OriginalMethod = originalMethod; _hoistedVariables = hoistedVariables; _synthesizedLocalOrdinals = synthesizedLocalOrdinals; @@ -204,13 +179,6 @@ protected override BoundExpression FramePointer(SyntaxNode syntax, NamedTypeSymb protected void AddState(out int stateNumber, out GeneratedLabelSymbol resumeLabel) { stateNumber = _nextState++; - - if (_useFinalizerBookkeeping && !_hasFinalizerState) - { - _currentFinalizerState = _nextState++; - _hasFinalizerState = true; - } - AddState(stateNumber, out resumeLabel); } @@ -796,53 +764,24 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node) public override BoundNode VisitTryStatement(BoundTryStatement node) { var oldDispatches = _dispatches; - var oldFinalizerState = _currentFinalizerState; - var oldHasFinalizerState = _hasFinalizerState; _dispatches = null; - _currentFinalizerState = -1; - _hasFinalizerState = false; BoundBlock tryBlock = F.Block((BoundStatement)this.Visit(node.TryBlock)); GeneratedLabelSymbol dispatchLabel = null; if (_dispatches != null) { dispatchLabel = F.GenerateLabel("tryDispatch"); - if (_hasFinalizerState) - { - // cause the current finalizer state to arrive here and then "return false" - var finalizer = F.GenerateLabel("finalizer"); - _dispatches.Add(finalizer, new List() { _currentFinalizerState }); - var skipFinalizer = F.GenerateLabel("skipFinalizer"); - tryBlock = F.Block( - F.HiddenSequencePoint(), - Dispatch(), - F.Goto(skipFinalizer), - F.Label(finalizer), // code for the finalizer here - GenerateSetBothStates(StateMachineStates.NotStartedStateMachine), - GenerateReturn(false), - F.Label(skipFinalizer), - tryBlock); - } - else - { - tryBlock = F.Block( - F.HiddenSequencePoint(), - Dispatch(), - tryBlock); - } - if (oldDispatches == null) - { - Debug.Assert(!oldHasFinalizerState); - oldDispatches = new Dictionary>(); - } + tryBlock = F.Block( + F.HiddenSequencePoint(), + Dispatch(), + tryBlock); + oldDispatches ??= new Dictionary>(); oldDispatches.Add(dispatchLabel, new List(from kv in _dispatches.Values from n in kv orderby n select n)); } - _hasFinalizerState = oldHasFinalizerState; - _currentFinalizerState = oldFinalizerState; _dispatches = oldDispatches; ImmutableArray catchBlocks = this.VisitList(node.CatchBlocks);