From cc33c27d9c39e230f02778638dbb264a509988d0 Mon Sep 17 00:00:00 2001
From: Daniel Grunwald <daniel@danielgrunwald.de>
Date: Sun, 20 Aug 2017 18:45:46 +0200
Subject: [PATCH] ConditionDetection: Try to prefer 'break;' over other gotos

---
 .../IL/ControlFlow/ConditionDetection.cs      | 19 +++++++++++++++++--
 .../IL/Instructions/BlockContainer.cs         | 10 ++++++++++
 .../IL/Transforms/DelegateConstruction.cs     |  8 +-------
 .../Tests/TestCases/Pretty/Loops.cs           |  2 +-
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs
index 54c8edb2a..3de3e2baf 100644
--- a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs
+++ b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs
@@ -90,7 +90,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 
 		private void HandleIfInstruction(ControlFlowNode cfgNode, Block block, IfInstruction ifInst, ref ILInstruction exitInst)
 		{
-			if (IsBranchToLaterTarget(ifInst.TrueInst, exitInst)) {
+			if (ShouldSwapIfTargets(ifInst.TrueInst, exitInst)) {
 				// "if (c) goto lateBlock; goto earlierBlock;"
 				// -> "if (!c)" goto earlierBlock; goto lateBlock;
 				// This reordering should make the if structure correspond more closely to the original C# source code
@@ -219,10 +219,11 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				return inst;
 		}
 
-		bool IsBranchToLaterTarget(ILInstruction inst1, ILInstruction inst2)
+		bool ShouldSwapIfTargets(ILInstruction inst1, ILInstruction inst2)
 		{
 			Block block1 = null, block2 = null;
 			if (inst1.MatchBranch(out block1) && inst2.MatchBranch(out block2)) {
+				// prefer arranging stuff in IL order
 				return block1.ILRange.Start > block2.ILRange.Start;
 			}
 			BlockContainer container1, container2;
@@ -236,6 +237,20 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 					container2 = block2?.Parent as BlockContainer;
 				return container2 == null || container2.IsDescendantOf(container1);
 			}
+			if (inst1.MatchBranch(out block1) && inst2.MatchLeave(out container2)
+				&& block1.IncomingEdgeCount > 1)
+			{
+				// if (..) goto x; leave c;
+				// Unless x can be inlined, it's better to swap the order if the 'leave'
+				// has a chance to turn into a 'break;' or 'return;'
+				if (container2.Parent is ILFunction) {
+					return true; // return
+				}
+				if (container2.EntryPoint.IncomingEdgeCount > 1) {
+					// break
+					return BlockContainer.FindClosestContainer(inst2) == container2;
+				}
+			}
 			return false;
 		}
 
diff --git a/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs b/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs
index 8673f4791..260d7101c 100644
--- a/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs
+++ b/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs
@@ -213,5 +213,15 @@ namespace ICSharpCode.Decompiler.IL
 			Debug.Assert(postOrder[0] == Blocks[0]);
 			Blocks.ReplaceList(postOrder);
 		}
+
+		public static BlockContainer FindClosestContainer(ILInstruction inst)
+		{
+			while (inst != null) {
+				if (inst is BlockContainer bc)
+					return bc;
+				inst = inst.Parent;
+			}
+			return null;
+		}
 	}
 }
diff --git a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs
index 9a5f3f9cb..161af62ad 100644
--- a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs
+++ b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs
@@ -75,13 +75,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 
 		private BlockContainer FindBlockContainer(Block block)
 		{
-			ILInstruction current = block;
-			while (current != null) {
-				current = current.Parent;
-				if (current is BlockContainer)
-					return (BlockContainer)current;
-			}
-			throw new NotSupportedException();
+			return BlockContainer.FindClosestContainer(block) ?? throw new NotSupportedException();
 		}
 
 		bool IsSimpleDisplayClass(IMethod method)
diff --git a/ICSharpCode.Decompiler/Tests/TestCases/Pretty/Loops.cs b/ICSharpCode.Decompiler/Tests/TestCases/Pretty/Loops.cs
index 69874cabb..9c5c73b60 100644
--- a/ICSharpCode.Decompiler/Tests/TestCases/Pretty/Loops.cs
+++ b/ICSharpCode.Decompiler/Tests/TestCases/Pretty/Loops.cs
@@ -188,7 +188,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 					if (Condition("test")) {
 						if (Condition("continue"))
 							continue;
-						if (!Condition("break"))
+						if (!Condition("not-break"))
 							break;
 					}
 					Console.WriteLine("End of loop body");