diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index fe954d84d..a46325d2e 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -70,7 +70,7 @@ namespace ICSharpCode.Decompiler.CSharp new ILInlining(), new DetectPinnedRegions(), // must run after inlining but before non-critical control flow transforms new YieldReturnDecompiler(), // must run after inlining but before loop detection - new DetectExitPoints(), + new DetectExitPoints(canIntroduceExitForReturn: false), new BlockILTransform { PostOrderTransforms = { new ExpressionTransforms() // for RemoveDeadVariableInit @@ -109,6 +109,7 @@ namespace ICSharpCode.Decompiler.CSharp ) } }, + //new DetectExitPoints(canIntroduceExitForReturn: true), new DelegateConstruction(), }; } diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/ExitPoints.cs b/ICSharpCode.Decompiler/IL/ControlFlow/ExitPoints.cs index 320b4007f..43c43ffea 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/ExitPoints.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/ExitPoints.cs @@ -49,7 +49,14 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow { static readonly Nop ExitNotYetDetermined = new Nop(); static readonly Nop NoExit = new Nop(); - + + bool canIntroduceExitForReturn; + + public DetectExitPoints(bool canIntroduceExitForReturn) + { + this.canIntroduceExitForReturn = canIntroduceExitForReturn; + } + /// /// Gets the next instruction after is executed. /// Returns NoExit when the next instruction cannot be identified; @@ -150,7 +157,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow void HandleExit(ILInstruction inst) { - if (currentExit == ExitNotYetDetermined && !(inst is Leave l && l.IsLeavingFunction)) { + if (currentExit == ExitNotYetDetermined && CanIntroduceAsExit(inst)) { currentExit = inst; inst.ReplaceWith(new Leave(currentContainer) { ILRange = inst.ILRange }); } else if (CompatibleExitInstruction(inst, currentExit)) { @@ -158,6 +165,15 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow } } + private bool CanIntroduceAsExit(ILInstruction inst) + { + if (inst is Leave l && l.IsLeavingFunction) { + return canIntroduceExitForReturn; + } else { + return true; + } + } + protected internal override void VisitBranch(Branch inst) { if (!inst.TargetBlock.IsDescendantOf(currentContainer)) { diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs b/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs index 155a5aff7..a76a06cf7 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs @@ -84,12 +84,10 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow /// readonly Dictionary decompiledFinallyMethods = new Dictionary(); - /* /// - /// List of blocks that change the iterator state on block entry. + /// Temporary stores for 'yield break'. /// - readonly List<(int state, Block block)> stateChanges = new List<(int state, Block block)>(); - */ + readonly List returnStores = new List(); #region Run() method public void Run(ILFunction function, ILTransformContext context) @@ -105,6 +103,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow this.fieldToParameterMap.Clear(); this.finallyMethodToStateRange = null; this.decompiledFinallyMethods.Clear(); + this.returnStores.Clear(); if (!MatchEnumeratorCreationPattern(function)) return; BlockContainer newBody; @@ -138,6 +137,15 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow context.Step("Translate fields to local accesses", function); TranslateFieldsToLocalAccess(function, function, fieldToParameterMap); + if (returnStores.Count > 0) { + context.Step("Remove temporaries", function); + foreach (var store in returnStores) { + if (store.Variable.LoadCount == 0 && store.Variable.AddressCount == 0 && store.Parent is Block block) { + block.Instructions.Remove(store); + } + } + } + // Re-run control flow simplification over the newly constructed set of gotos, // and inlining because TranslateFieldsToLocalAccess() might have opened up new inlining opportunities. function.RunTransforms(CSharpDecompiler.EarlyILTransforms(), context); @@ -440,7 +448,6 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow newBody.Blocks.Add(new Block { ILRange = oldBody.Blocks[blockIndex].ILRange }); } // convert contents of blocks - var ssaDefs = new Dictionary(); for (int i = 0; i < oldBody.Blocks.Count; i++) { var oldBlock = oldBody.Blocks[i]; @@ -472,23 +479,10 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow // Break up the basic block on a call to a finally method // (this allows us to consider each block individually for try-finally reconstruction) newBlock = SplitBlock(newBlock, oldInst); - } else if (oldInst.MatchReturn(out value)) { - if (value.MatchLdLoc(out var v)) { - ssaDefs.TryGetValue(v, out value); - } - if (value != null && value.MatchLdcI4(0)) { - // yield break - newBlock.Instructions.Add(new Leave(newBody) { ILRange = oldInst.ILRange }); - } else { - newBlock.Instructions.Add(new InvalidBranch("Unexpected return in MoveNext()")); - } - break; // we're done with this basic block - } else if (oldInst.MatchStLoc(out var v, out value) && v.IsSingleDefinition) { - ssaDefs.Add(v, value); } // copy over the instruction to the new block - UpdateBranchTargets(oldInst); newBlock.Instructions.Add(oldInst); + UpdateBranchTargets(oldInst); } } @@ -556,6 +550,21 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow leave.TargetContainer = newBody; } break; + case Return ret: + ILInstruction value = ret.Value; + if (value.MatchLdLoc(out var v) && v.IsSingleDefinition + && v.StoreInstructions.SingleOrDefault() is StLoc stloc) + { + returnStores.Add(stloc); + value = stloc.Value; + } + if (value.MatchLdcI4(0)) { + // yield break + ret.ReplaceWith(new Leave(newBody) { ILRange = ret.ILRange }); + } else { + ret.ReplaceWith(new InvalidBranch("Unexpected return in MoveNext()") { ILRange = ret.ILRange }); + } + break; } foreach (var child in inst.Children) { UpdateBranchTargets(child); diff --git a/ICSharpCode.Decompiler/IL/ILVariable.cs b/ICSharpCode.Decompiler/IL/ILVariable.cs index 66e47a773..f78703f11 100644 --- a/ICSharpCode.Decompiler/IL/ILVariable.cs +++ b/ICSharpCode.Decompiler/IL/ILVariable.cs @@ -19,6 +19,7 @@ using System; using System.Collections.Generic; using ICSharpCode.Decompiler.TypeSystem; +using System.Diagnostics; namespace ICSharpCode.Decompiler.IL { @@ -125,7 +126,7 @@ namespace ICSharpCode.Decompiler.IL /// public int LoadCount => LoadInstructions.Count; - readonly List loadInstructions = new List(); + readonly List loadInstructions = new List(); /// /// List of ldloc instructions referencing this variable. @@ -133,10 +134,11 @@ namespace ICSharpCode.Decompiler.IL /// /// This list is automatically updated when adding/removing ldloc instructions from the ILAst. /// - public IReadOnlyList LoadInstructions => loadInstructions; + public IReadOnlyList LoadInstructions => loadInstructions; /// - /// Number of store instructions referencing this variable. + /// Number of store instructions referencing this variable, + /// plus 1 if HasInitialValue. /// /// Stores are: /// @@ -161,7 +163,8 @@ namespace ICSharpCode.Decompiler.IL /// stloc /// TryCatchHandler (assigning the exception variable) /// PinnedRegion (assigning the pointer variable) - /// initial values () + /// initial values () -- however, there is no instruction for + /// the initial value, so it is not contained in the store list. /// /// /// @@ -187,30 +190,27 @@ namespace ICSharpCode.Decompiler.IL /// public IReadOnlyList AddressInstructions => addressInstructions; - internal void AddLoadInstruction(ILoadInstruction inst) => inst.IndexInLoadInstructionList = AddInstruction(loadInstructions, inst); + internal void AddLoadInstruction(LdLoc inst) => inst.IndexInLoadInstructionList = AddInstruction(loadInstructions, inst); internal void AddStoreInstruction(IStoreInstruction inst) => inst.IndexInStoreInstructionList = AddInstruction(storeInstructions, inst); internal void AddAddressInstruction(LdLoca inst) => inst.IndexInAddressInstructionList = AddInstruction(addressInstructions, inst); - internal void RemoveLoadInstruction(ILoadInstruction inst) => RemoveInstruction(loadInstructions, inst.IndexInLoadInstructionList); - internal void RemoveStoreInstruction(IStoreInstruction inst) => RemoveInstruction(storeInstructions, inst.IndexInStoreInstructionList); - internal void RemoveAddressInstruction(LdLoca inst) => RemoveInstruction(addressInstructions, inst.IndexInAddressInstructionList); + internal void RemoveLoadInstruction(LdLoc inst) => RemoveInstruction(loadInstructions, inst.IndexInLoadInstructionList, inst); + internal void RemoveStoreInstruction(IStoreInstruction inst) => RemoveInstruction(storeInstructions, inst.IndexInStoreInstructionList, inst); + internal void RemoveAddressInstruction(LdLoca inst) => RemoveInstruction(addressInstructions, inst.IndexInAddressInstructionList, inst); - int AddInstruction(List list, T inst) where T : IInstructionWithVariableOperand + int AddInstruction(List list, T inst) where T : class, IInstructionWithVariableOperand { list.Add(inst); return list.Count - 1; } - void RemoveInstruction(List list, int index) where T : IInstructionWithVariableOperand - { - if (list.Count > 1) { - int indexToMove = list.Count - 1; - list[index] = list[indexToMove]; - list[index].IndexInVariableInstructionMapping = index; - list.RemoveAt(indexToMove); - } else { - list.RemoveAt(0); - } + void RemoveInstruction(List list, int index, T inst) where T : class, IInstructionWithVariableOperand + { + Debug.Assert(list[index] == inst); + int indexToMove = list.Count - 1; + list[index] = list[indexToMove]; + list[index].IndexInVariableInstructionMapping = index; + list.RemoveAt(indexToMove); } bool hasInitialValue; @@ -321,12 +321,12 @@ namespace ICSharpCode.Decompiler.IL int IndexInStoreInstructionList { get; set; } } - public interface ILoadInstruction : IInstructionWithVariableOperand + interface ILoadInstruction : IInstructionWithVariableOperand { int IndexInLoadInstructionList { get; set; } } - public interface IAddressInstruction : IInstructionWithVariableOperand + interface IAddressInstruction : IInstructionWithVariableOperand { int IndexInAddressInstructionList { get; set; } } diff --git a/ICSharpCode.Decompiler/Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler/Tests/ICSharpCode.Decompiler.Tests.csproj index 744d56f33..cf7e4722b 100644 --- a/ICSharpCode.Decompiler/Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler/Tests/ICSharpCode.Decompiler.Tests.csproj @@ -147,7 +147,7 @@ - + diff --git a/ICSharpCode.Decompiler/Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler/Tests/PrettyTestRunner.cs index 0720476fd..316b0dcd5 100644 --- a/ICSharpCode.Decompiler/Tests/PrettyTestRunner.cs +++ b/ICSharpCode.Decompiler/Tests/PrettyTestRunner.cs @@ -104,7 +104,7 @@ namespace ICSharpCode.Decompiler.Tests } [Test, Ignore("Not implemented")] - public void YieldReturn([ValueSource("defaultOptions")] CompilerOptions cscOptions) + public void YieldReturnTest([ValueSource("defaultOptions")] CompilerOptions cscOptions) { Run(cscOptions: cscOptions); } diff --git a/ICSharpCode.Decompiler/Tests/TestCases/Pretty/YieldReturn.cs b/ICSharpCode.Decompiler/Tests/TestCases/Pretty/YieldReturnTest.cs similarity index 98% rename from ICSharpCode.Decompiler/Tests/TestCases/Pretty/YieldReturn.cs rename to ICSharpCode.Decompiler/Tests/TestCases/Pretty/YieldReturnTest.cs index 64339cdca..508030482 100644 --- a/ICSharpCode.Decompiler/Tests/TestCases/Pretty/YieldReturn.cs +++ b/ICSharpCode.Decompiler/Tests/TestCases/Pretty/YieldReturnTest.cs @@ -21,7 +21,7 @@ using System.Collections.Generic; namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty { - public class YieldReturn + public class YieldReturnTest { int fieldOnThis; @@ -213,6 +213,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty // yield return is not allowed in catch, but yield break is yield break; } + yield return 1; } finally { Console.WriteLine("Finally"); } @@ -228,21 +229,24 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty } catch { Console.WriteLine("Catch"); } + yield return 1; } finally { Console.WriteLine("Finally"); } } - public static IEnumerable YieldBreakInTryFinallyInTryFinally() + public static IEnumerable YieldBreakInTryFinallyInTryFinally(bool b) { try { yield return 0; try { Console.WriteLine("In Try"); - yield break; + if (b) + yield break; } finally { Console.WriteLine("Inner Finally"); } + yield return 1; } finally { Console.WriteLine("Finally"); }