Attempt to improve control flow detection

ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@@ -45,6 +45,7 @@ namespace ICSharpCode.Decompiler.CSharp
 
 		List<IILTransform> ilTransforms = new List<IILTransform> {
 			new ControlFlowSimplification(),
+			new ILInlining(), // temporary pass, just to make the ILAst easier to read while debugging loop detection
 			new LoopDetection(),
 			new IntroduceExitPoints(),
 			new ConditionDetection(),

ICSharpCode.Decompiler/CSharp/StatementBuilder.cs

@@ -204,32 +204,38 @@ namespace ICSharpCode.Decompiler.CSharp
 				blockStatement.Add(Convert(block.FinalInstruction));
 			return blockStatement;
 		}
-		
+
 		protected internal override Statement VisitBlockContainer(BlockContainer container)
 		{
 			if (container.EntryPoint.IncomingEdgeCount > 1) {
 				var oldContinueTarget = continueTarget;
 				var oldContinueCount = continueCount;
 				var oldBreakTarget = breakTarget;
-				continueTarget = container.EntryPoint;
-				continueCount = 0;
-				breakTarget = container;
-				var blockStatement = ConvertBlockContainer(container, true);
-				Debug.Assert(continueCount < container.EntryPoint.IncomingEdgeCount);
-				Debug.Assert(blockStatement.Statements.First() is LabelStatement);
-				if (container.EntryPoint.IncomingEdgeCount == continueCount + 1) {
-					// Remove the entrypoint label if all jumps to the label were replaced with 'continue;' statements
-					blockStatement.Statements.First().Remove();
-				}
+				var loop = ConvertLoop(container);
 				continueTarget = oldContinueTarget;
 				continueCount = oldContinueCount;
 				breakTarget = oldBreakTarget;
-				return new WhileStatement(new PrimitiveExpression(true), blockStatement);
+				return loop;
 			} else {
 				return ConvertBlockContainer(container, false);
 			}
 		}
 		
+		WhileStatement ConvertLoop(BlockContainer container)
+		{
+			continueTarget = container.EntryPoint;
+			continueCount = 0;
+			breakTarget = container;
+			var blockStatement = ConvertBlockContainer(container, true);
+			Debug.Assert(continueCount < container.EntryPoint.IncomingEdgeCount);
+			Debug.Assert(blockStatement.Statements.First() is LabelStatement);
+			if (container.EntryPoint.IncomingEdgeCount == continueCount + 1) {
+				// Remove the entrypoint label if all jumps to the label were replaced with 'continue;' statements
+				blockStatement.Statements.First().Remove();
+			}
+			return new WhileStatement(new PrimitiveExpression(true), blockStatement);
+		}
+		
 		BlockStatement ConvertBlockContainer(BlockContainer container, bool isLoop)
 		{
 			BlockStatement blockStatement = new BlockStatement();

ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs

@@ -77,6 +77,16 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			// usually an IfInstruction with a branch:
 			IfInstruction ifInst = block.Instructions.ElementAtOrDefault(block.Instructions.Count - 2) as IfInstruction;
 			if (ifInst != null && ifInst.FalseInst.OpCode == OpCode.Nop) {
+				if (IsBranchToLaterTarget(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
+					block.Instructions[block.Instructions.Count - 1] = ifInst.TrueInst;
+					ifInst.TrueInst = exitInst;
+					exitInst = block.Instructions.Last();
+					ifInst.Condition = new LogicNot(ifInst.Condition);
+				}
+				
 				ILInstruction trueExitInst;
 				if (IsUsableBranchToChild(cfgNode, ifInst.TrueInst)) {
 					// "if (...) goto targetBlock; exitInst;"
@@ -136,6 +146,15 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				targetBlock.Instructions.Clear();
 			}
 		}
+
+		bool IsBranchToLaterTarget(ILInstruction inst1, ILInstruction inst2)
+		{
+			Block block1, block2;
+			if (inst1.MatchBranch(out block1) && inst2.MatchBranch(out block2)) {
+				return block1.ILRange.Start > block2.ILRange.Start;
+			}
+			return false;
+		}
 		
 		bool IsUsableBranchToChild(ControlFlowNode cfgNode, ILInstruction potentialBranchInstruction)
 		{

ICSharpCode.Decompiler/IL/ControlFlow/IntroduceExitPoints.cs

@@ -97,9 +97,9 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 		{
 			if (currentExit == null) {
 				currentExit = inst;
-				inst.ReplaceWith(new Leave(currentContainer));
+				inst.ReplaceWith(new Leave(currentContainer) { ILRange = inst.ILRange });
 			} else if (ConditionDetection.CompatibleExitInstruction(inst, currentExit)) {
-				inst.ReplaceWith(new Leave(currentContainer));
+				inst.ReplaceWith(new Leave(currentContainer) { ILRange = inst.ILRange });
 			}
 		}
 		

ICSharpCode.Decompiler/IL/Instructions/PatternMatching.cs

@@ -67,12 +67,34 @@ namespace ICSharpCode.Decompiler.IL
 			return false;
 		}
 		
+		public bool MatchBranch(out Block targetBlock)
+		{
+			var inst = this as Branch;
+			if (inst != null) {
+				targetBlock = inst.TargetBlock;
+				return true;
+			}
+			targetBlock = null;
+			return false;
+		}
+		
 		public bool MatchBranch(Block targetBlock)
 		{
 			var inst = this as Branch;
 			return inst != null && inst.TargetBlock == targetBlock;
 		}
 		
+		public bool MatchLeave(out BlockContainer targetContainer)
+		{
+			var inst = this as Leave;
+			if (inst != null) {
+				targetContainer = inst.TargetContainer;
+				return true;
+			}
+			targetContainer = null;
+			return false;
+		}
+		
 		public bool MatchLeave(BlockContainer targetContainer)
 		{
 			var inst = this as Leave;

ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@@ -283,7 +283,10 @@ namespace ICSharpCode.Decompiler.IL
 				case OpCode.Return:
 					return parent == next;
 				case OpCode.IfInstruction:
-					return parent == next || (parent.OpCode == OpCode.LogicNot && parent.Parent == next);
+					while (parent.OpCode == OpCode.LogicNot) {
+						parent = parent.Parent;
+					}
+					return parent == next;
 				case OpCode.SwitchInstruction:
 					return parent == next || (parent.OpCode == OpCode.Sub && parent.Parent == next);
 				default:

ICSharpCode.Decompiler/Tests/Loops.cs

@@ -122,5 +122,68 @@ public class Loops
 		}
 		return i;
 	}
+	
+	bool Condition(string arg)
+	{
+		Console.WriteLine("Condition: " + arg);
+		return false;
+	}
+	
+	public void WhileLoop()
+	{
+		Console.WriteLine("Initial");
+		if (Condition("if")) {
+			while (Condition("while")) {
+				Console.WriteLine("Loop Body");
+				if (Condition("test")) {
+					if (Condition("continue"))
+						continue;
+					if (!Condition("break"))
+						break;
+				}
+				Console.WriteLine("End of loop body");
+			}
+			Console.WriteLine("After loop");
+		}
+		Console.WriteLine("End of method");
+	}
+	
+	public void DoWhileLoop()
+	{
+		Console.WriteLine("Initial");
+		if (Condition("if")) {
+			do {
+				Console.WriteLine("Loop Body");
+				if (Condition("test")) {
+					if (Condition("continue"))
+						continue;
+					if (!Condition("break"))
+						break;
+				}
+				Console.WriteLine("End of loop body");
+			} while (Condition("while"));
+			Console.WriteLine("After loop");
+		}
+		Console.WriteLine("End of method");
+	}
+	
+	public void ForLoop()
+	{
+		Console.WriteLine("Initial");
+		if (Condition("if")) {
+			for (int i = 0; Condition("for"); i++) {
+				Console.WriteLine("Loop Body");
+				if (Condition("test")) {
+					if (Condition("continue"))
+						continue;
+					if (!Condition("break"))
+						break;
+				}
+				Console.WriteLine("End of loop body");
+			}
+			Console.WriteLine("After loop");
+		}
+		Console.WriteLine("End of method");
+	}
 }