Fix inline-block creation.

ICSharpCode.Decompiler/IL/Instructions.cs

@@ -528,10 +528,6 @@ namespace ICSharpCode.Decompiler.IL
 	public sealed partial class Branch : SimpleInstruction
 	{
 		public override StackType ResultType { get { return StackType.Void; } }
-		protected override InstructionFlags ComputeFlags()
-		{
-			return InstructionFlags.EndPointUnreachable | InstructionFlags.MayBranch;
-		}
 		public override T AcceptVisitor<T>(ILVisitor<T> visitor)
 		{
 			return visitor.VisitBranch(this);

ICSharpCode.Decompiler/IL/Instructions.tt

@@ -60,7 +60,7 @@
 		new OpCode("bit.not", "Bitwise NOT", Unary),
 		new OpCode("arglist", "Retrieves the RuntimeArgumentHandle.", NoArguments, ResultType("O")),
 		new OpCode("br", "Unconditional branch. <c>goto target;</c>",
-			CustomClassName("Branch"), NoArguments, CustomConstructor, UnconditionalBranch, MayBranch),
+			CustomClassName("Branch"), NoArguments, CustomConstructor, UnconditionalBranch, MayBranch, CustomComputeFlags),
 		// TODO: get rid of endfinally, we can represent those with a normal branch to the end of the filter/finally block instead
 		new OpCode("endfinally", "Marks the end of an finally, fault or exception filter block.",
 			CustomClassName("EndFinally"), NoArguments, UnconditionalBranch, MayBranch),

ICSharpCode.Decompiler/IL/Instructions/Branch.cs

@@ -25,6 +25,14 @@ using System.Threading.Tasks;
 
 namespace ICSharpCode.Decompiler.IL
 {
+	/// <summary>
+	/// Unconditional branch. <c>goto target;</c>
+	/// </summary>
+	/// <remarks>
+	/// Phase-1 execution of a branch is a no-op.
+	/// Phase-2 execution removes PopCount elements from the evaluation stack
+	/// and jumps to the target block.
+	/// </remarks>
 	partial class Branch : SimpleInstruction
 	{
 		readonly int targetILOffset;
@@ -32,8 +40,6 @@ namespace ICSharpCode.Decompiler.IL
 		
 		/// <summary>
 		/// Pops the specified number of arguments from the evaluation stack during the branching operation.
-		/// Note that the Branch instruction does not set InstructionFlags.MayPop -- the pop instead is considered
-		/// to happen after the branch was taken.
 		/// </summary>
 		public int PopCount;
 		
@@ -42,6 +48,16 @@ namespace ICSharpCode.Decompiler.IL
 			this.targetILOffset = targetILOffset;
 		}
 		
+		protected override InstructionFlags ComputeFlags()
+		{
+			var flags = InstructionFlags.MayBranch | InstructionFlags.EndPointUnreachable;
+			if (PopCount > 0) {
+				// the branch pop happens during phase-2, so don't use MayPop
+				flags |= InstructionFlags.MayWriteEvaluationStack;
+			}
+			return flags;
+		}
+		
 		public int TargetILOffset {
 			get { return targetBlock != null ? targetBlock.ILRange.Start : targetILOffset; }
 		}

ICSharpCode.Decompiler/IL/Instructions/ILFunction.cs

@@ -37,8 +37,10 @@ namespace ICSharpCode.Decompiler.IL
 		public override void WriteTo(ITextOutput output)
 		{
 			output.Write(OpCode);
-			output.Write(' ');
-			Method.WriteTo(output);
+			if (Method != null) {
+				output.Write(' ');
+				Method.WriteTo(output);
+			}
 			output.WriteLine(" {");
 			output.Indent();
 			

ICSharpCode.Decompiler/IL/TransformingVisitor.cs

@@ -94,7 +94,11 @@ namespace ICSharpCode.Decompiler.IL
 				stack.didInline = false;
 				stack.error = false;
 				inst = inst.Inline(InstructionFlags.None, stack);
-				Debug.Assert(stack.error == inst.HasFlag(InstructionFlags.MayPeek | InstructionFlags.MayPop));
+				// An error implies that a peek or pop instruction wasn't replaced
+				// But even if we replaced all peek/pop instructions, we might have replaced them with
+				// another peek or pop instruction, so MayPeek/MayPop might still be set after
+				// we finish without error!
+				Debug.Assert(!stack.error || inst.HasFlag(InstructionFlags.MayPeek | InstructionFlags.MayPop));
 			} while (stack.didInline); // repeat transformations when something was inlined
 			return inst;
 		}
@@ -105,44 +109,51 @@ namespace ICSharpCode.Decompiler.IL
 			List<ILInstruction> output = new List<ILInstruction>();
 			for (int i = 0; i < block.Instructions.Count; i++) {
 				var inst = block.Instructions[i];
-				inst = DoInline(stack, inst); 
-				if (inst.HasFlag(InstructionFlags.MayBranch 
-				                 | InstructionFlags.MayPeek | InstructionFlags.MayPop
+				inst = DoInline(stack, inst);
+				if (inst.HasFlag(InstructionFlags.MayBranch | InstructionFlags.MayPop
 				                 | InstructionFlags.MayReadEvaluationStack | InstructionFlags.MayWriteEvaluationStack)) {
 					// Values currently on the stack might be used on both sides of the branch,
 					// so we can't inline them.
 					// We also have to flush the stack if the instruction still accesses the evaluation stack,
 					// no matter whether in phase-1 or phase-2.
 					FlushInstructionStack(stack, output);
-				}
-				if (inst.ResultType == StackType.Void) {
-					// We cannot directly push instructions onto the stack if they don't produce
-					// a result.
-					if (!stack.error && stack.Count > 0) {
-						// Wrap the instruction on top of the stack into an inline block,
-						// and append our void-typed instruction to the end of that block.
-						// TODO: I think this is wrong now that we changed the inline block semantics;
-						// we need to re-think how to build inline blocks.
-						var headInst = stack.Pop();
-						var nestedBlock = headInst as Block ?? new Block {
+				} else if (inst.ResultType == StackType.Void && stack.Count > 0) {
+					// For void instructions on non-empty stack, we can create a new inline block (or add to an existing one)
+					// This works even when inst involves Peek.
+					ILInstruction headInst = stack.Pop();
+					Block inlineBlock = headInst as Block;
+					if (inlineBlock == null || inlineBlock.FinalInstruction.OpCode != OpCode.Pop) {
+						inlineBlock = new Block {
 							Instructions = { headInst },
-							ILRange = headInst.ILRange
+							ILRange = new Interval(headInst.ILRange.Start, headInst.ILRange.Start),
+							FinalInstruction = new Pop(headInst.ResultType)
 						};
-						nestedBlock.Instructions.Add(inst);
-						stack.Push(nestedBlock);
-					} else {
-						// We can't move incomplete instructions into a nested block
-						// or the instruction stack was empty
-						FlushInstructionStack(stack, output);
-						output.Add(inst);
 					}
+					inlineBlock.Instructions.Add(inst);
+					inst = inlineBlock;
+				}
+				if (inst.HasFlag(InstructionFlags.MayPeek)) {
+					// Prevent instruction from being inlined if it was peeked at.
+					FlushInstructionStack(stack, output);
+				}
+				if (inst.ResultType == StackType.Void) {
+					// We can't add void instructions to the stack, so flush the stack
+					// and directly add the instruction to the output.
+					FlushInstructionStack(stack, output);
+					output.Add(inst);
 				} else {
 					// Instruction has a result, so we can push it on the stack normally
 					stack.Push(inst);
 				}
 			}
 			// Allow inlining into the final instruction
-			block.FinalInstruction = DoInline(stack, block.FinalInstruction);
+			if (block.FinalInstruction.OpCode == OpCode.Pop && stack.Count > 0 && IsInlineBlock(stack.Peek())) {
+				// Don't inline an inline block into the final pop instruction:
+				// doing so would result in infinite recursion.
+			} else {
+				// regular inlining into the final instruction
+				block.FinalInstruction = DoInline(stack, block.FinalInstruction);
+			}
 			FlushInstructionStack(stack, output);
 			block.Instructions.ReplaceList(output);
 			if (!(block.Parent is BlockContainer)) {
@@ -151,12 +162,32 @@ namespace ICSharpCode.Decompiler.IL
 			return block;
 		}
 
+		bool IsInlineBlock(ILInstruction inst)
+		{
+			Block block = inst as Block;
+			return block != null && block.FinalInstruction.OpCode == OpCode.Pop;
+		}
+		
 		void FlushInstructionStack(Stack<ILInstruction> stack, List<ILInstruction> output)
 		{
-			output.AddRange(stack.Reverse());
+			foreach (var inst in stack.Reverse()) {
+				AddToOutput(inst, output);
+			}
 			stack.Clear();
 		}
 		
+		void AddToOutput(ILInstruction inst, List<ILInstruction> output)
+		{
+			// Unpack inline blocks that would become direct children of the parent block
+			if (IsInlineBlock(inst)) {
+				foreach (var nestedInst in ((Block)inst).Instructions) {
+					AddToOutput(nestedInst, output);
+				}
+			} else {
+				output.Add(inst);
+			}
+		}
+		
 		protected internal override ILInstruction VisitBlockContainer(BlockContainer container)
 		{
 			container.TransformChildren(this);

ICSharpCode.Decompiler/Tests/ILTransforms/Inlining.cs

@@ -16,6 +16,7 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 // DEALINGS IN THE SOFTWARE.
 using System;
+using System.Diagnostics;
 using ICSharpCode.Decompiler.IL;
 using NUnit.Framework;
 using ICSharpCode.Decompiler.Tests.Helpers;
@@ -91,5 +92,38 @@ namespace ICSharpCode.Decompiler.Tests.ILTransforms
 				f.Body.ToString()
 			);
 		}
+		
+		[Test]
+		public void BuildInlineBlock()
+		{
+			var f = MakeFunction(
+				new Block {
+					Instructions = {
+						new LdcI4(1),
+						new LdcI4(2),
+						new Call(TypeSystem.Action<int>()) { Arguments = { new Peek(StackType.I4) } },
+						new Call(TypeSystem.Action<int>()) { Arguments = { new Peek(StackType.I4) } },
+						new Call(TypeSystem.Action<int, int>()) { Arguments = { new Pop(StackType.I4), new Pop(StackType.I4) } }
+					}
+				});
+			f.AcceptVisitor(new TransformingVisitor());
+			Debug.WriteLine(f.ToString());
+			Assert.AreEqual(
+				new Call(TypeSystem.Action<int, int>()) {
+					Arguments = {
+						new LdcI4(1),
+						new Block {
+							Instructions = {
+								new LdcI4(2),
+								new Call(TypeSystem.Action<int>()) { Arguments = { new Peek(StackType.I4) } },
+								new Call(TypeSystem.Action<int>()) { Arguments = { new Peek(StackType.I4) } },
+							},
+							FinalInstruction = new Pop(StackType.I4)
+						}
+					}
+				}.ToString(),
+				f.Body.ToString()
+			);
+		}
 	}
 }