Remove 'loop' instruction; just use a block container.

ICSharpCode.Decompiler/CSharp/StatementBuilder.cs

@@ -74,11 +74,6 @@ namespace ICSharpCode.Decompiler.CSharp
 			return new IfElseStatement(condition, trueStatement, falseStatement);
 		}
 		
-		protected internal override Statement VisitLoop(Loop inst)
-		{
-			return new WhileStatement(new PrimitiveExpression(true), Convert(inst.Body));
-		}
-
 		protected internal override Statement VisitBranch(Branch inst)
 		{
 			return new GotoStatement(inst.TargetLabel);

ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj

@@ -95,7 +95,6 @@
     <Compile Include="IL\Instructions\ILInstruction.cs" />
     <Compile Include="IL\Instructions\InstructionCollection.cs" />
     <Compile Include="IL\Instructions\LocalVarInstructions.cs" />
-    <Compile Include="IL\Instructions\Loop.cs" />
     <Compile Include="IL\Instructions\MemoryInstructions.cs" />
     <Compile Include="IL\Instructions\PatternMatching.cs" />
     <Compile Include="IL\Instructions\Return.cs" />

ICSharpCode.Decompiler/IL/Instructions.cs

@@ -70,8 +70,6 @@ namespace ICSharpCode.Decompiler.IL
 		EndFinally,
 		/// <summary>If statement / conditional expression. <c>if (condition) trueExpr else falseExpr</c></summary>
 		IfInstruction,
-		/// <summary>Infinite loop</summary>
-		Loop,
 		/// <summary>Try-catch statement.</summary>
 		TryCatch,
 		/// <summary>Catch handler within a try-catch statement.</summary>
@@ -608,37 +606,6 @@ namespace ICSharpCode.Decompiler.IL
 		}
 	}
 
-	/// <summary>Infinite loop</summary>
-	public sealed partial class Loop : ILInstruction
-	{
-		public Loop(ILInstruction body) : base(OpCode.Loop)
-		{
-			this.Body = body;
-		}
-		ILInstruction body;
-		public ILInstruction Body {
-			get { return this.body; }
-			set {
-				ValidateChild(value);
-				SetChildInstruction(ref this.body, value);
-			}
-		}
-		public override IEnumerable<ILInstruction> Children {
-			get {
-				yield return this.body;
-			}
-		}
-		public override void TransformChildren(ILVisitor<ILInstruction> visitor)
-		{
-			this.Body = this.body.AcceptVisitor(visitor);
-		}
-		public override StackType ResultType { get { return StackType.Void; } }
-		public override T AcceptVisitor<T>(ILVisitor<T> visitor)
-		{
-			return visitor.VisitLoop(this);
-		}
-	}
-
 	/// <summary>Try-catch statement.</summary>
 	public sealed partial class TryCatch : TryInstruction
 	{
@@ -2137,10 +2104,6 @@ namespace ICSharpCode.Decompiler.IL
 		{
 			return Default(inst);
 		}
-		protected internal virtual T VisitLoop(Loop inst)
-		{
-			return Default(inst);
-		}
 		protected internal virtual T VisitTryCatch(TryCatch inst)
 		{
 			return Default(inst);
@@ -2419,7 +2382,6 @@ namespace ICSharpCode.Decompiler.IL
 			"br",
 			"endfinally",
 			"if",
-			"loop",
 			"try.catch",
 			"try.catch.handler",
 			"try.finally",

ICSharpCode.Decompiler/IL/Instructions.tt

@@ -71,11 +71,6 @@
 				new ChildInfo("trueInst"),
 				new ChildInfo("falseInst"),
 			}), CustomConstructor, CustomComputeFlags, CustomWriteTo),
-		new OpCode("loop", "Infinite loop",
-			CustomChildren(new []{
-				new ChildInfo("body")
-			}),
-			CustomComputeFlags, CustomWriteTo, ResultType("Void")),
 		new OpCode("try.catch", "Try-catch statement.",
 			BaseClass("TryInstruction"), CustomConstructor, CustomComputeFlags, CustomWriteTo),
 		new OpCode("try.catch.handler", "Catch handler within a try-catch statement.",

ICSharpCode.Decompiler/IL/Instructions/Loop.cs

@@ -1,50 +0,0 @@
-// 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
-// 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.
-using System;
-
-namespace ICSharpCode.Decompiler.IL
-{
-	partial class Loop
-	{
-		public override void WriteTo(ITextOutput output)
-		{
-			output.Write("loop ");
-			body.WriteTo(output);
-		}
-		
-		protected override InstructionFlags ComputeFlags()
-		{
-			return Block.Phase1Boundary(body.Flags)
-				| InstructionFlags.EndPointUnreachable
-				| InstructionFlags.SideEffect; // an infinite loop is a side effect
-		}
-		
-		internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context)
-		{
-			return this;
-		}
-		
-		internal override void TransformStackIntoVariables(TransformStackIntoVariablesState state)
-		{
-			var initialVariables = state.Variables.Clone();
-			Body = Body.Inline(InstructionFlags.None, state);
-			Body.TransformStackIntoVariables(state);
-			state.MergeVariables(initialVariables, state.Variables);
-		}
-	}
-}

ICSharpCode.Decompiler/IL/Transforms/LoopDetection.cs

@@ -126,8 +126,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				}
 			}
 			if (loop != null) {
-				// loop now is the union of all natural loops with loop head h
+				// loop now is the union of all natural loops with loop head h.
-				Debug.Assert(loop[0] == h);
+				// Sort blocks in the loop in reverse post-order to make the output look a bit nicer.
+				// (if the loop doesn't contain nested loops, this is a topological sort)
+				loop.Sort((a, b) => b.PostOrderNumber.CompareTo(a.PostOrderNumber));
+				Debug.Assert(loop[0] == h); // TODO: is this guaranteed after sorting?
 				foreach (var node in loop) {
 					node.Visited = false; // reset visited flag so that we can find nested loops
 					Debug.Assert(h.Dominates(node), "The loop body must be dominated by the loop head");
@@ -148,15 +151,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			Block oldEntryPoint = (Block)loop[0].UserData;
 			BlockContainer oldContainer = (BlockContainer)oldEntryPoint.Parent;
 			
-			BlockContainer body = new BlockContainer();
+			BlockContainer loopContainer = new BlockContainer();
 			Block newEntryPoint = new Block();
-			body.Blocks.Add(newEntryPoint);
+			loopContainer.Blocks.Add(newEntryPoint);
 			// Move contents of oldEntryPoint to newEntryPoint
 			// (we can't move the block itself because it might be the target of branch instructions outside the loop)
 			newEntryPoint.Instructions.ReplaceList(oldEntryPoint.Instructions);
 			newEntryPoint.FinalInstruction = oldEntryPoint.FinalInstruction;
 			newEntryPoint.ILRange = oldEntryPoint.ILRange;
-			oldEntryPoint.Instructions.ReplaceList(new[] { new Loop(body) });
+			oldEntryPoint.Instructions.ReplaceList(new[] { loopContainer });
 			oldEntryPoint.FinalInstruction = new Nop();
 			
 			// Move other blocks into the loop body: they're all dominated by the loop header,
@@ -164,43 +167,16 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			for (int i = 1; i < loop.Count; i++) {
 				Block block = (Block)loop[i].UserData;
 				Debug.Assert(block.Parent == oldContainer);
-				body.Blocks.Add(block);
+				loopContainer.Blocks.Add(block);
 			}
 			// Remove all blocks that were moved into the body from the old container
 			oldContainer.Blocks.RemoveAll(b => b.Parent != oldContainer);
 			
 			// Rewrite branches within the loop from oldEntryPoint to newEntryPoint:
-			foreach (var branch in body.Descendants.OfType<Branch>()) {
+			foreach (var branch in loopContainer.Descendants.OfType<Branch>()) {
 				if (branch.TargetBlock == oldEntryPoint)
 					branch.TargetBlock = newEntryPoint;
 			}
-			
-			// Because we use extended basic blocks, not just basic blocks, it's possible
-			// that we moved too many instructions into the loop body.
-			// In cases where those additional instructions branch, that's not really a problem
-			// (and is more readable with the instructions being inside the loop, anyways)
-			// But in cases where those additional instructions fall through to the end of
-			// the block, they previously had the semantics of falling out of the oldContainer,
-			// thus leaving the loop.
-			// Now they just fall out of the body block container, and thus get executed repeatedly.
-			// To fix up this semantic difference, we'll patch up these blocks to branch to
-			// a new empty block in the old container.
-			Block fallthrough_helper_block = null;
-			foreach (var block in body.Blocks) {
-				if (!block.HasFlag(InstructionFlags.EndPointUnreachable)) {
-					if (block.FinalInstruction.OpCode != OpCode.Nop) {
-						// move final instruction into the block
-						block.Instructions.Add(new Void(block.FinalInstruction));
-						block.FinalInstruction = new Nop();
-					}
-					if (fallthrough_helper_block == null) {
-						fallthrough_helper_block = new Block();
-						oldContainer.Blocks.Add(fallthrough_helper_block);
-					}
-					block.Instructions.Add(new Branch(fallthrough_helper_block));
-				}
-				Debug.Assert(block.HasFlag(InstructionFlags.EndPointUnreachable));
-			}
 		}
 	}
 }