From 2f33d380fe3bd4f2aa6fa3c108c91facebc41fb4 Mon Sep 17 00:00:00 2001 From: ElektroKill Date: Wed, 11 Aug 2021 19:36:44 +0200 Subject: [PATCH] Merge PR #2425: Keep return statements around in original form for ConditionDetection --- .../TestCases/Pretty/ExceptionHandling.cs | 22 ++ .../TestCases/Pretty/Loops.cs | 21 ++ .../TestCases/Pretty/ReduceNesting.cs | 1 - .../CSharp/CSharpDecompiler.cs | 5 +- .../CSharp/StatementBuilder.cs | 23 +- .../ICSharpCode.Decompiler.csproj | 1 + .../IL/ControlFlow/ConditionDetection.cs | 2 - .../IL/ControlFlow/ExitPoints.cs | 223 ++++++++++-------- .../IL/ControlFlow/RemoveRedundantReturn.cs | 115 +++++++++ .../IL/Instructions/InstructionCollection.cs | 26 +- .../IL/Instructions/Leave.cs | 6 +- .../IL/Transforms/HighLevelLoopTransform.cs | 5 +- .../IL/Transforms/ReduceNestingTransform.cs | 12 +- 13 files changed, 331 insertions(+), 131 deletions(-) create mode 100644 ICSharpCode.Decompiler/IL/ControlFlow/RemoveRedundantReturn.cs diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ExceptionHandling.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ExceptionHandling.cs index b2a766c647..b76d0db7d9 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ExceptionHandling.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ExceptionHandling.cs @@ -196,6 +196,28 @@ public void ThrowInFinally() } } + internal void EarlyReturnInTryBlock(bool a, bool b) + { + try + { + if (a) + { + Console.WriteLine("a"); + } + else if (b) + { + // #2379: The only goto-free way of representing this code is to use a return statement + return; + } + + Console.WriteLine("a || !b"); + } + finally + { + Console.WriteLine("finally"); + } + } + #if ROSLYN || !OPT // TODO Non-Roslyn compilers create a second while loop inside the try, by inverting the if // This is fixed in the non-optimised version by the enabling the RemoveDeadCode flag diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.cs index f742c6dec9..0e6aacdf6c 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.cs @@ -906,5 +906,26 @@ public void MergeAroundContinue() } Console.WriteLine("end"); } + + public void ForEachInSwitch(int i, IEnumerable args) + { + switch (i) + { + + case 1: + Console.WriteLine("one"); + break; + case 2: + { + foreach (string arg in args) + { + Console.WriteLine(arg); + } + break; + } + default: + throw new NotImplementedException(); + } + } } } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ReduceNesting.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ReduceNesting.cs index 8d9b4c841f..8bf735a068 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ReduceNesting.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ReduceNesting.cs @@ -174,7 +174,6 @@ public void NestedSwitchIf() Console.WriteLine("case 1"); return; } - if (B(1)) { Console.WriteLine(1); } diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index d5f11e82cf..ea64450d21 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -86,7 +86,7 @@ public static List GetILTransforms() new YieldReturnDecompiler(), // must run after inlining but before loop detection new AsyncAwaitDecompiler(), // must run after inlining but before loop detection new DetectCatchWhenConditionBlocks(), // must run after inlining but before loop detection - new DetectExitPoints(canIntroduceExitForReturn: false), + new DetectExitPoints(), new LdLocaDupInitObjTransform(), new EarlyExpressionTransforms(), // RemoveDeadVariableInit must run after EarlyExpressionTransforms so that stobj(ldloca V, ...) @@ -111,7 +111,7 @@ public static List GetILTransforms() } }, // re-run DetectExitPoints after loop detection - new DetectExitPoints(canIntroduceExitForReturn: true), + new DetectExitPoints(), new BlockILTransform { // per-block transforms PostOrderTransforms = { new ConditionDetection(), @@ -153,6 +153,7 @@ public static List GetILTransforms() new TransformDisplayClassUsage(), new HighLevelLoopTransform(), new ReduceNestingTransform(), + new RemoveRedundantReturn(), new IntroduceDynamicTypeOnLocals(), new IntroduceNativeIntTypeOnLocals(), new AssignVariableNames(), diff --git a/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs b/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs index 3795dc5d40..3fbc0a6266 100644 --- a/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs @@ -583,7 +583,7 @@ Statement TransformToForeach(UsingInstruction inst, Expression resource) var enumeratorVar = inst.Variable; // If there's another BlockContainer nested in this container and it only has one child block, unwrap it. // If there's an extra leave inside the block, extract it into optionalReturnAfterLoop. - var loopContainer = UnwrapNestedContainerIfPossible(container, out var optionalReturnAfterLoop); + var loopContainer = UnwrapNestedContainerIfPossible(container, out var optionalLeaveAfterLoop); // Detect whether we're dealing with a while loop with multiple embedded statements. if (loopContainer.Kind != ContainerKind.While) return null; @@ -713,22 +713,22 @@ Statement TransformToForeach(UsingInstruction inst, Expression resource) // If there was an optional return statement, return it as well. // If there were labels or any other statements in the whileLoopBlock, move them after the foreach // loop. - if (optionalReturnAfterLoop != null || whileLoopBlock.Statements.Count > 1) + if (optionalLeaveAfterLoop != null || whileLoopBlock.Statements.Count > 1) { var block = new BlockStatement { Statements = { foreachStmt } }; - if (optionalReturnAfterLoop != null) + if (optionalLeaveAfterLoop != null) { - block.Statements.Add(optionalReturnAfterLoop.AcceptVisitor(this)); + block.Statements.Add(optionalLeaveAfterLoop.AcceptVisitor(this)); } if (whileLoopBlock.Statements.Count > 1) { block.Statements.AddRange(whileLoopBlock.Statements .Skip(1) - .SkipWhile(s => s.Annotations.Any(a => a == optionalReturnAfterLoop)) + .SkipWhile(s => s.Annotations.Any(a => a == optionalLeaveAfterLoop)) .Select(SyntaxExtensions.Detach)); } return block; @@ -794,10 +794,10 @@ private bool IsDynamicCastToIEnumerable(Expression expr, out Expression dynamicE /// if the value has no side-effects. /// Otherwise returns the unmodified container. /// - /// If the leave is a return and has no side-effects, we can move the return out of the using-block and put it after the loop, otherwise returns null. - BlockContainer UnwrapNestedContainerIfPossible(BlockContainer container, out Leave optionalReturnInst) + /// If the leave is a return/break and has no side-effects, we can move the return out of the using-block and put it after the loop, otherwise returns null. + BlockContainer UnwrapNestedContainerIfPossible(BlockContainer container, out Leave optionalLeaveInst) { - optionalReturnInst = null; + optionalLeaveInst = null; // Check block structure: if (container.Blocks.Count != 1) return container; @@ -809,10 +809,11 @@ BlockContainer UnwrapNestedContainerIfPossible(BlockContainer container, out Lea // If the leave has no value, just unwrap the BlockContainer. if (leave.MatchLeave(container)) return nestedContainer; - // If the leave is a return, we can move the return out of the using-block and put it after the loop + // If the leave is a return/break, we can move it out of the using-block and put it after the loop // (but only if the value doesn't have side-effects) - if (leave.IsLeavingFunction && SemanticHelper.IsPure(leave.Value.Flags)) { - optionalReturnInst = leave; + if (SemanticHelper.IsPure(leave.Value.Flags)) + { + optionalLeaveInst = leave; return nestedContainer; } return container; diff --git a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj index b03a14ec9f..90f3435723 100644 --- a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj +++ b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj @@ -64,6 +64,7 @@ + diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs index 07a34d0384..99bd9f3867 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs @@ -16,7 +16,6 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. -using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -352,7 +351,6 @@ internal static void InvertIf(Block block, IfInstruction ifInst, ILTransformCont Debug.Assert(ifInst.Parent == block); //assert then block terminates - var trueExitInst = GetExit(ifInst.TrueInst); var exitInst = GetExit(block); context.Step($"InvertIf at IL_{ifInst.StartILOffset:x4}", ifInst); diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/ExitPoints.cs b/ICSharpCode.Decompiler/IL/ControlFlow/ExitPoints.cs index 4d74019787..2f30c82459 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/ExitPoints.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/ExitPoints.cs @@ -1,14 +1,14 @@ // Copyright (c) 2014 Daniel Grunwald -// +// // Permission is hereby granted, free of charge, to any person obtaining a copy of this // software and associated documentation files (the "Software"), to deal in the Software // without restriction, including without limitation the rights to use, copy, modify, merge, // publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons // to whom the Software is furnished to do so, subject to the following conditions: -// +// // The above copyright notice and this permission notice shall be included in all copies or // substantial portions of the Software. -// +// // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, // INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR // PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE @@ -16,34 +16,36 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. -using System; +#nullable enable + +using System.Collections.Generic; using System.Diagnostics; -using ICSharpCode.Decompiler.IL.Transforms; +using System.Linq; using System.Threading; -using System.Collections.Generic; +using ICSharpCode.Decompiler.IL.Transforms; namespace ICSharpCode.Decompiler.IL.ControlFlow { /// /// Detect suitable exit points for BlockContainers. - /// + /// /// An "exit point" is an instruction that causes control flow /// to leave the container (a branch or leave instruction). - /// + /// /// If an "exit point" instruction is placed immediately following a /// block container, each equivalent exit point within the container /// can be replaced with a "leave container" instruction. - /// + /// /// This transform performs this replacement: any exit points /// equivalent to the exit point following the container are /// replaced with a leave instruction. /// Additionally, if the container is not yet followed by an exit point, - /// but has room to introduce such an exit point (i.e. iff the container's + /// but has room to introduce such an exit point (i.e. iff the container's /// end point is currently unreachable), we pick one of the non-return /// exit points within the container, move it to the position following the /// container, and replace all instances within the container with a leave /// instruction. - /// + /// /// This makes it easier for the following transforms to construct /// control flow that falls out of blocks instead of using goto/break statements. /// @@ -52,13 +54,6 @@ public class DetectExitPoints : ILVisitor, IILTransform static readonly Nop ExitNotYetDetermined = new Nop { Comment = "ExitNotYetDetermined" }; static readonly Nop NoExit = new Nop { Comment = "NoExit" }; - 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; @@ -66,16 +61,20 @@ public DetectExitPoints(bool canIntroduceExitForReturn) /// internal static ILInstruction GetExit(ILInstruction inst) { - SlotInfo slot = inst.SlotInfo; - if (slot == Block.InstructionSlot) { - Block block = (Block)inst.Parent; + SlotInfo? slot = inst.SlotInfo; + if (slot == Block.InstructionSlot) + { + Block block = (Block)inst.Parent!; return block.Instructions.ElementAtOrDefault(inst.ChildIndex + 1) ?? ExitNotYetDetermined; - } else if (slot == TryInstruction.TryBlockSlot - || slot == TryCatchHandler.BodySlot - || slot == TryCatch.HandlerSlot - || slot == PinnedRegion.BodySlot) + } + else if (slot == TryInstruction.TryBlockSlot + || slot == TryCatchHandler.BodySlot + || slot == TryCatch.HandlerSlot + || slot == PinnedRegion.BodySlot + || slot == UsingInstruction.BodySlot + || slot == LockInstruction.BodySlot) { - return GetExit(inst.Parent); + return GetExit(inst.Parent!); } return NoExit; } @@ -102,30 +101,53 @@ internal static bool CompatibleExitInstruction(ILInstruction exit1, ILInstructio } } - CancellationToken cancellationToken; - BlockContainer currentContainer; + class ContainerContext + { + public readonly BlockContainer Container; - /// - /// The instruction that will be executed next after leaving the currentContainer. - /// ExitNotYetDetermined means the container is last in its parent block, and thus does not - /// yet have any leave instructions. This means we can move any exit instruction of - /// our choice our of the container and replace it with a leave instruction. - /// - ILInstruction currentExit; + /// + /// The instruction that will be executed next after leaving the Container. + /// ExitNotYetDetermined means the container is last in its parent block, and thus does not + /// yet have any leave instructions. This means we can move any exit instruction of + /// our choice our of the container and replace it with a leave instruction. + /// + public readonly ILInstruction CurrentExit; - /// - /// If currentExit==ExitNotYetDetermined, holds the list of potential exit instructions. - /// After the currentContainer was visited completely, one of these will be selected as exit instruction. - /// - List potentialExits; + /// + /// If currentExit==ExitNotYetDetermined, holds the list of potential exit instructions. + /// After the currentContainer was visited completely, one of these will be selected as exit instruction. + /// + public readonly List? PotentialExits = null; + public ContainerContext(BlockContainer container, ILInstruction currentExit) + { + this.Container = container; + this.CurrentExit = currentExit; + this.PotentialExits = (currentExit == ExitNotYetDetermined ? new List() : null); + } + + public void HandleExit(ILInstruction inst) + { + if (this.CurrentExit == ExitNotYetDetermined && this.Container.LeaveCount == 0) + { + this.PotentialExits!.Add(inst); + } + else if (CompatibleExitInstruction(inst, this.CurrentExit)) + { + inst.ReplaceWith(new Leave(this.Container).WithILRange(inst)); + } + } + } + + CancellationToken cancellationToken; readonly List blocksPotentiallyMadeUnreachable = new List(); + readonly Stack containerStack = new Stack(); public void Run(ILFunction function, ILTransformContext context) { cancellationToken = context.CancellationToken; - currentExit = NoExit; blocksPotentiallyMadeUnreachable.Clear(); + containerStack.Clear(); function.AcceptVisitor(this); // It's possible that there are unreachable code blocks which we only // detect as such during exit point detection. @@ -136,6 +158,7 @@ public void Run(ILFunction function, ILTransformContext context) } } blocksPotentiallyMadeUnreachable.Clear(); + containerStack.Clear(); } static bool IsInfiniteLoop(Block block) @@ -150,30 +173,29 @@ protected override void Default(ILInstruction inst) foreach (var child in inst.Children) child.AcceptVisitor(this); } - + protected internal override void VisitBlockContainer(BlockContainer container) { - var oldExit = currentExit; - var oldContainer = currentContainer; - var oldPotentialExits = potentialExits; var thisExit = GetExit(container); - currentExit = thisExit; - currentContainer = container; - potentialExits = (thisExit == ExitNotYetDetermined ? new List() : null); + var stackEntry = new ContainerContext(container, thisExit); + containerStack.Push(stackEntry); base.VisitBlockContainer(container); - if (thisExit == ExitNotYetDetermined && potentialExits.Count > 0) { + if (stackEntry.PotentialExits?.Any(i => i.IsConnected) ?? false) + { // This transform determined an exit point. - currentExit = ChooseExit(potentialExits); - foreach (var exit in potentialExits) { - if (CompatibleExitInstruction(currentExit, exit)) { - exit.ReplaceWith(new Leave(currentContainer).WithILRange(exit)); + var newExit = ChooseExit(stackEntry.PotentialExits.Where(i => i.IsConnected)); + Debug.Assert(!newExit.MatchLeave(container)); + foreach (var exit in stackEntry.PotentialExits) + { + if (exit.IsConnected && CompatibleExitInstruction(newExit, exit)) + { + exit.ReplaceWith(new Leave(container).WithILRange(exit)); } } - Debug.Assert(!currentExit.MatchLeave(currentContainer)); ILInstruction inst = container; // traverse up to the block (we'll always find one because GetExit // only returns ExitNotYetDetermined if there's a block) - while (inst.Parent.OpCode != OpCode.Block) + while (inst.Parent!.OpCode != OpCode.Block) inst = inst.Parent; Block block = (Block)inst.Parent; if (block.HasFlag(InstructionFlags.EndPointUnreachable)) { @@ -181,27 +203,33 @@ protected internal override void VisitBlockContainer(BlockContainer container) // we still have an unreachable endpoint. // The appended currentExit instruction would not be reachable! // This happens in test case ExceptionHandling.ThrowInFinally() - if (currentExit is Branch b) { + if (newExit is Branch b) + { blocksPotentiallyMadeUnreachable.Add(b.TargetBlock); } - } else { - block.Instructions.Add(currentExit); } - } else { - Debug.Assert(thisExit == currentExit); + else + { + block.Instructions.Add(newExit); + } + } + if (containerStack.Pop() != stackEntry) + { + Debug.Fail("containerStack got imbalanced"); } - currentExit = oldExit; - currentContainer = oldContainer; - potentialExits = oldPotentialExits; } - static ILInstruction ChooseExit(List potentialExits) + static ILInstruction ChooseExit(IEnumerable potentialExits) { - ILInstruction first = potentialExits[0]; - if (first is Leave l && l.IsLeavingFunction) { - for (int i = 1; i < potentialExits.Count; i++) { - var exit = potentialExits[i]; - if (!(exit is Leave l2 && l2.IsLeavingFunction)) + using var enumerator = potentialExits.GetEnumerator(); + enumerator.MoveNext(); + ILInstruction first = enumerator.Current; + if (first is Leave { IsLeavingFunction: true }) + { + while (enumerator.MoveNext()) + { + var exit = enumerator.Current; + if (!(exit is Leave { IsLeavingFunction: true })) return exit; } } @@ -217,33 +245,13 @@ protected internal override void VisitBlock(Block block) } } - void HandleExit(ILInstruction inst) - { - if (currentExit == ExitNotYetDetermined && CanIntroduceAsExit(inst)) { - potentialExits.Add(inst); - } else if (CompatibleExitInstruction(inst, currentExit)) { - inst.ReplaceWith(new Leave(currentContainer).WithILRange(inst)); - } - } - - private bool CanIntroduceAsExit(ILInstruction inst) - { - if (currentContainer.LeaveCount > 0) { - // if we're re-running on a block container that already has an exit, - // we can't introduce any additional exits - return false; - } - if (inst is Leave l && l.IsLeavingFunction) { - return canIntroduceExitForReturn; - } else { - return true; - } - } - protected internal override void VisitBranch(Branch inst) { - if (!inst.TargetBlock.IsDescendantOf(currentContainer)) { - HandleExit(inst); + foreach (var entry in containerStack) + { + if (inst.TargetBlock.IsDescendantOf(entry.Container)) + break; + entry.HandleExit(inst); } } @@ -252,7 +260,32 @@ protected internal override void VisitLeave(Leave inst) base.VisitLeave(inst); if (!inst.Value.MatchNop()) return; - HandleExit(inst); + foreach (var entry in containerStack) + { + if (inst.TargetContainer == entry.Container) + break; + if (inst.IsLeavingFunction || inst.TargetContainer.Kind != ContainerKind.Normal) + { + if (entry.Container.Kind == ContainerKind.Normal) + { + // Don't transform a `return`/`break` into a leave for try-block containers (or similar). + // It's possible that those can be turned into fallthrough later, but + // it might not work out and then we would be left with a `goto`. + // But continue searching the container stack, it might be possible to + // turn the `return` into a `break` instead. + } + else + { + // return; could turn to break; + entry.HandleExit(inst); + break; // but only for the innermost loop/switch + } + } + else + { + entry.HandleExit(inst); + } + } } } } diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/RemoveRedundantReturn.cs b/ICSharpCode.Decompiler/IL/ControlFlow/RemoveRedundantReturn.cs new file mode 100644 index 0000000000..d6cbe2d1ca --- /dev/null +++ b/ICSharpCode.Decompiler/IL/ControlFlow/RemoveRedundantReturn.cs @@ -0,0 +1,115 @@ +// Copyright (c) Daniel Grunwald +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +#nullable enable + +using System.Linq; + +using ICSharpCode.Decompiler.IL.Transforms; + +namespace ICSharpCode.Decompiler.IL.ControlFlow +{ + /// + /// Similar to , but acts only on leave instructions + /// leaving the whole function (return/yield break) that can be made implicit + /// without using goto. + /// + class RemoveRedundantReturn : IILTransform + { + public void Run(ILFunction function, ILTransformContext context) + { + foreach (var lambda in function.Descendants.OfType()) + { + if (lambda.Body is BlockContainer c && ((lambda.AsyncReturnType ?? lambda.ReturnType).Kind == TypeSystem.TypeKind.Void || lambda.IsIterator)) + { + Block lastBlock = c.Blocks.Last(); + if (lastBlock.Instructions.Last() is Leave { IsLeavingFunction: true }) + { + ConvertReturnToFallthrough(lastBlock.Instructions.SecondToLastOrDefault()); + } + else + { + if (ConvertReturnToFallthrough(lastBlock.Instructions.Last())) + { + lastBlock.Instructions.Add(new Leave(c)); + } + } + } + } + } + + private static bool ConvertReturnToFallthrough(ILInstruction? inst) + { + bool result = false; + switch (inst) + { + case BlockContainer c when c.Kind == ContainerKind.Normal: + // body of try block, or similar: recurse into last instruction in container + // Note: no need to handle loops/switches here; those already were handled by DetectExitPoints + Block lastBlock = c.Blocks.Last(); + if (lastBlock.Instructions.Last() is Leave { IsLeavingFunction: true, Value: Nop } leave) + { + leave.TargetContainer = c; + result = true; + } + else if (ConvertReturnToFallthrough(lastBlock.Instructions.Last())) + { + lastBlock.Instructions.Add(new Leave(c)); + result = true; + } + break; + case TryCatch tryCatch: + result |= ConvertReturnToFallthrough(tryCatch.TryBlock); + foreach (var h in tryCatch.Handlers) + { + result |= ConvertReturnToFallthrough(h.Body); + } + break; + case TryFinally tryFinally: + result |= ConvertReturnToFallthrough(tryFinally.TryBlock); + break; + case LockInstruction lockInst: + result |= ConvertReturnToFallthrough(lockInst.Body); + break; + case UsingInstruction usingInst: + result |= ConvertReturnToFallthrough(usingInst.Body); + break; + case PinnedRegion pinnedRegion: + result |= ConvertReturnToFallthrough(pinnedRegion.Body); + break; + case IfInstruction ifInstruction: + result |= ConvertReturnToFallthrough(ifInstruction.TrueInst); + result |= ConvertReturnToFallthrough(ifInstruction.FalseInst); + break; + case Block block when block.Kind == BlockKind.ControlFlow: + { + var lastInst = block.Instructions.LastOrDefault(); + if (lastInst is Leave { IsLeavingFunction: true, Value: Nop }) + { + block.Instructions.RemoveAt(block.Instructions.Count - 1); + result = true; + lastInst = block.Instructions.LastOrDefault(); + } + result |= ConvertReturnToFallthrough(lastInst); + break; + } + } + return result; + } + } +} diff --git a/ICSharpCode.Decompiler/IL/Instructions/InstructionCollection.cs b/ICSharpCode.Decompiler/IL/Instructions/InstructionCollection.cs index 63d5fd97d3..85b6ae0709 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/InstructionCollection.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/InstructionCollection.cs @@ -16,6 +16,8 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. +#nullable enable + using System; using System.Collections.Generic; using System.Diagnostics; @@ -71,9 +73,9 @@ public Enumerator GetEnumerator() /// public struct Enumerator : IEnumerator { - #if DEBUG - ILInstruction parentInstruction; - #endif +#if DEBUG + ILInstruction? parentInstruction; +#endif readonly List list; int pos; @@ -138,7 +140,7 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() /// Runs in O(1) if the item can be found using the Parent/ChildIndex properties. /// Otherwise, runs in O(N). /// - public int IndexOf(T item) + public int IndexOf(T? item) { if (item == null) { // InstructionCollection can't contain nulls @@ -160,7 +162,7 @@ public int IndexOf(T item) /// This method searches the list. /// Usually it's more efficient to test item.Parent instead! /// - public bool Contains(T item) + public bool Contains(T? item) { return IndexOf(item) >= 0; } @@ -374,7 +376,7 @@ public T First() return list[0]; } - public T FirstOrDefault() + public T? FirstOrDefault() { return list.Count > 0 ? list[0] : null; } @@ -383,18 +385,18 @@ public T Last() { return list[list.Count - 1]; } - - public T LastOrDefault() + + public T? LastOrDefault() { return list.Count > 0 ? list[list.Count - 1] : null; } - - public T SecondToLastOrDefault() + + public T? SecondToLastOrDefault() { return list.Count > 1 ? list[list.Count - 2] : null; } - - public T ElementAtOrDefault(int index) + + public T? ElementAtOrDefault(int index) { if (index >= 0 && index < list.Count) return list[index]; diff --git a/ICSharpCode.Decompiler/IL/Instructions/Leave.cs b/ICSharpCode.Decompiler/IL/Instructions/Leave.cs index c266ca5066..c35e1c0955 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/Leave.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/Leave.cs @@ -83,11 +83,15 @@ public string TargetLabel { } /// - /// Gets whether the leave instruction is leaving the whole ILFunction. + /// Gets whether the leave instruction is directly leaving the whole ILFunction. /// (TargetContainer == main container of the function). /// /// This is only valid for functions returning void (representing value-less "return;"), /// and for iterators (representing "yield break;"). + /// + /// Note: returns false for leave instructions that indirectly leave the function + /// (e.g. leaving a try block, and the try-finally construct is immediately followed + /// by another leave instruction) /// public bool IsLeavingFunction { get { return targetContainer?.Parent is ILFunction; } diff --git a/ICSharpCode.Decompiler/IL/Transforms/HighLevelLoopTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/HighLevelLoopTransform.cs index b119f0c625..1b2411c42e 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/HighLevelLoopTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/HighLevelLoopTransform.cs @@ -20,7 +20,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; -using System.Text; + using ICSharpCode.Decompiler.IL.ControlFlow; using ICSharpCode.Decompiler.Util; @@ -38,7 +38,8 @@ public void Run(ILFunction function, ILTransformContext context) { this.context = context; - foreach (var loop in function.Descendants.OfType()) { + foreach (BlockContainer loop in function.Descendants.OfType()) + { if (loop.Kind != ContainerKind.Loop) continue; if (MatchWhileLoop(loop, out var condition, out var loopBody)) { diff --git a/ICSharpCode.Decompiler/IL/Transforms/ReduceNestingTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/ReduceNestingTransform.cs index bed03fae88..d76e31d5d4 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ReduceNestingTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ReduceNestingTransform.cs @@ -90,17 +90,19 @@ private void Visit(Block block, Block continueTarget, ILInstruction nextInstruct var inst = block.Instructions[i]; // the next instruction to be executed. Transformations will change the next instruction, so this is a method instead of a variable - ILInstruction NextInsn() => i + 1 < block.Instructions.Count ? block.Instructions[i + 1] : nextInstruction; - - switch (inst) { + ILInstruction NextInsn() => block.Instructions.ElementAtOrDefault(i + 1) ?? nextInstruction; + + switch (inst) + { case BlockContainer container: // visit the contents of the container Visit(container, continueTarget); // reduce nesting in switch blocks if (container.Kind == ContainerKind.Switch && - CanDuplicateExit(NextInsn(), continueTarget, out var keywordExit1) && - ReduceSwitchNesting(block, container, keywordExit1)) { + CanDuplicateExit(NextInsn(), continueTarget, out var keywordExit1) && + ReduceSwitchNesting(block, container, keywordExit1)) + { RemoveRedundantExit(block, nextInstruction); } break;