Put switch instructions into their own BlockContainer.

This removes the need for "goto case" in the ILAst as we can just branch to the appropriate block.
It also means we can support "break;" within switch using the "leave" instruction (otherwise we'd have to introduce yet another special kind of jump).

ICSharpCode.Decompiler/CSharp/StatementBuilder.cs

@@ -98,11 +98,17 @@ namespace ICSharpCode.Decompiler.CSharp
 			}
 			return new CaseLabel(exprBuilder.ConvertConstantValue(new ConstantResolveResult(type, value), allowImplicitConversion: true));
 		}
-		
+
 		protected internal override Statement VisitSwitchInstruction(SwitchInstruction inst)
 		{
+			return TranslateSwitch(null, inst);
+		}
+
+		SwitchStatement TranslateSwitch(BlockContainer switchContainer, SwitchInstruction inst)
+		{
+			Debug.Assert(switchContainer.EntryPoint.IncomingEdgeCount == 1);
 			var oldBreakTarget = breakTarget;
-			breakTarget = null; // 'break' within a switch would only leave the switch
+			breakTarget = switchContainer; // 'break' within a switch would only leave the switch
 
 			TranslatedExpression value;
 			var strToInt = inst.Value as StringToInt;
@@ -134,7 +140,28 @@ namespace ICSharpCode.Decompiler.CSharp
 				ConvertSwitchSectionBody(astSection, section.Body);
 				stmt.SwitchSections.Add(astSection);
 			}
-			
+			if (switchContainer != null) {
+				// Translate any remaining blocks:
+				var lastSectionStatements = stmt.SwitchSections.Last().Statements;
+				foreach (var block in switchContainer.Blocks.Skip(1)) {
+					lastSectionStatements.Add(new LabelStatement { Label = block.Label });
+					foreach (var nestedInst in block.Instructions) {
+						var nestedStmt = Convert(nestedInst);
+						if (nestedStmt is BlockStatement b) {
+							foreach (var nested in b.Statements)
+								lastSectionStatements.Add(nested.Detach());
+						} else {
+							lastSectionStatements.Add(nestedStmt);
+						}
+					}
+					Debug.Assert(block.FinalInstruction.OpCode == OpCode.Nop);
+				}
+				if (endContainerLabels.TryGetValue(switchContainer, out string label)) {
+					lastSectionStatements.Add(new LabelStatement { Label = label });
+					lastSectionStatements.Add(new BreakStatement());
+				}
+			}
+
 			breakTarget = oldBreakTarget;
 			return stmt;
 		}
@@ -471,6 +498,8 @@ namespace ICSharpCode.Decompiler.CSharp
 				continueCount = oldContinueCount;
 				breakTarget = oldBreakTarget;
 				return loop;
+			} else if (container.EntryPoint.Instructions.Count == 1 && container.EntryPoint.Instructions[0] is SwitchInstruction switchInst) {
+				return TranslateSwitch(container, switchInst);
 			} else {
 				return ConvertBlockContainer(container, false);
 			}

ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs

@@ -624,17 +624,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis
 			state = afterSections;
 			DebugEndPoint(inst);
 		}
-
+		
-		protected internal override void VisitGotoCase(GotoCase inst)
-		{
-			// Handling goto case here is tricky:
-			//  We'll need a fixed-point iteration for SwitchInstruction similar to BlockContainer,
-			//  and we'll need to handle GotoCase like Branch, including stuff like ProcessBranchesLeavingTryFinally().
-			// Hopefully we won't need a data-flow analysis in the decompiler pipeline after 'goto case' instructions
-			// are already introduced.
-			throw new NotImplementedException();
-		}
-
 		protected internal override void VisitYieldReturn(YieldReturn inst)
 		{
 			DebugStartPoint(inst);

ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs

@@ -65,10 +65,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			Debug.Assert(block.Instructions.Last().HasFlag(InstructionFlags.EndPointUnreachable));
 
 			ILInstruction exitInst = block.Instructions.Last();
-			if (exitInst is SwitchInstruction switchInst) {
+			
-				HandleSwitchInstruction(cfgNode, block, switchInst);
-			}
-
 			// Previous-to-last instruction might have conditional control flow,
 			// usually an IfInstruction with a branch:
 			IfInstruction ifInst = block.Instructions.SecondToLastOrDefault() as IfInstruction;
@@ -287,22 +284,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				&& targetBlock.IncomingEdgeCount == 1 && targetBlock.FinalInstruction.OpCode == OpCode.Nop
 				&& cfgNode.Dominates(context.ControlFlowGraph.GetNode(targetBlock));
 		}
-
+		
-		private void HandleSwitchInstruction(ControlFlowNode cfgNode, Block block, SwitchInstruction sw)
-		{
-			// First, move blocks into the switch section
-			foreach (var section in sw.Sections) {
-				if (IsUsableBranchToChild(cfgNode, section.Body)) {
-					// case ...: goto targetBlock;
-					var targetBlock = ((Branch)section.Body).TargetBlock;
-					targetBlock.Remove();
-					section.Body = targetBlock;
-				}
-			}
-			// TODO: find a common exit point among the cases, and if possible inline that into this block.
-			sw.Sections.ReplaceList(sw.Sections.OrderBy(s => s.Body.ILRange.Start));
-		}
-
 		private bool IsBranchOrLeave(ILInstruction inst)
 		{
 			switch (inst) {

ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.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.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
@@ -57,6 +58,11 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			// Because this is a post-order block transform, we can assume that
 			// any nested loops within this loop have already been constructed.
 
+			if (block.Instructions.Last() is SwitchInstruction switchInst) {
+				// Switch instructions support "break;" just like loops
+				DetectSwitchBody(block, switchInst);
+			}
+
 			ControlFlowNode h = context.ControlFlowNode; // CFG node for our potential loop head
 			Debug.Assert(h.UserData == block);
 			Debug.Assert(!TreeTraversal.PreOrder(h, n => n.DominatorTreeChildren).Any(n => n.Visited));
@@ -101,7 +107,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				ConstructLoop(loop, exitPoint);
 			}
 		}
-		
+
 		/// <summary>
 		/// Recurse into the dominator tree and find back edges/natural loops.
 		/// </summary>
@@ -412,13 +418,25 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			// 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[] { loopContainer });
 			if (exitTargetBlock != null)
 				oldEntryPoint.Instructions.Add(new Branch(exitTargetBlock));
-			oldEntryPoint.FinalInstruction = new Nop();
+
-			
+			MoveBlocksIntoContainer(loop, loopContainer);
+
+			// Rewrite branches within the loop from oldEntryPoint to newEntryPoint:
+			foreach (var branch in loopContainer.Descendants.OfType<Branch>()) {
+				if (branch.TargetBlock == oldEntryPoint) {
+					branch.TargetBlock = newEntryPoint;
+				} else if (branch.TargetBlock == exitTargetBlock) {
+					branch.ReplaceWith(new Leave(loopContainer) { ILRange = branch.ILRange });
+				}
+			}
+		}
+
+		private void MoveBlocksIntoContainer(List<ControlFlowNode> loop, BlockContainer loopContainer)
+		{
 			// Move other blocks into the loop body: they're all dominated by the loop header,
 			// and thus cannot be the target of branch instructions outside the loop.
 			for (int i = 1; i < loop.Count; i++) {
@@ -440,12 +458,48 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				Block block = (Block)loop[i].UserData;
 				Debug.Assert(block.IsDescendantOf(loopContainer));
 			}
+		}
+
+		private void DetectSwitchBody(Block block, SwitchInstruction switchInst)
+		{
+			Debug.Assert(block.Instructions.Last() == switchInst);
+			ControlFlowNode h = context.ControlFlowNode; // CFG node for our potential loop head
+			Debug.Assert(h.UserData == block);
+			Debug.Assert(!TreeTraversal.PreOrder(h, n => n.DominatorTreeChildren).Any(n => n.Visited));
+
+			var nodesInSwitch = new List<ControlFlowNode>();
+			nodesInSwitch.Add(h);
+			h.Visited = true;
+			ExtendLoop(h, nodesInSwitch, out var exitPoint);
+
+			context.Step("Create BlockContainer for switch", switchInst);
+			// 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)
+			nodesInSwitch.Sort((a, b) => b.PostOrderNumber.CompareTo(a.PostOrderNumber));
+			Debug.Assert(nodesInSwitch[0] == h);
+			foreach (var node in nodesInSwitch) {
+				node.Visited = false; // reset visited flag so that we can find outer loops
+				Debug.Assert(h.Dominates(node) || !node.IsReachable, "The switch body must be dominated by the switch head");
+			}
+
+			BlockContainer switchContainer = new BlockContainer();
+			Block newEntryPoint = new Block();
+			newEntryPoint.ILRange = switchInst.ILRange;
+			switchContainer.Blocks.Add(newEntryPoint);
+			newEntryPoint.Instructions.Add(switchInst);
+			block.Instructions[block.Instructions.Count - 1] = switchContainer;
+
+			Block exitTargetBlock = (Block)exitPoint?.UserData;
+			if (exitTargetBlock != null) {
+				block.Instructions.Add(new Branch(exitTargetBlock));
+			}
+
+			MoveBlocksIntoContainer(nodesInSwitch, switchContainer);
+
 			// Rewrite branches within the loop from oldEntryPoint to newEntryPoint:
-			foreach (var branch in loopContainer.Descendants.OfType<Branch>()) {
+			foreach (var branch in switchContainer.Descendants.OfType<Branch>()) {
-				if (branch.TargetBlock == oldEntryPoint) {
+				if (branch.TargetBlock == exitTargetBlock) {
-					branch.TargetBlock = newEntryPoint;
+					branch.ReplaceWith(new Leave(switchContainer) { ILRange = branch.ILRange });
-				} else if (branch.TargetBlock == exitTargetBlock) {
-					branch.ReplaceWith(new Leave(loopContainer) { ILRange = branch.ILRange });
 				}
 			}
 		}

ICSharpCode.Decompiler/IL/Instructions.cs

@@ -63,8 +63,6 @@ namespace ICSharpCode.Decompiler.IL
 		SwitchInstruction,
 		/// <summary>Switch section within a switch statement</summary>
 		SwitchSection,
-		/// <summary>Unconditional branch to switch section. <c>goto case target;</c></summary>
-		GotoCase,
 		/// <summary>Try-catch statement.</summary>
 		TryCatch,
 		/// <summary>Catch handler within a try-catch statement.</summary>
@@ -1407,40 +1405,6 @@ namespace ICSharpCode.Decompiler.IL
 	}
 }
 namespace ICSharpCode.Decompiler.IL
-{
-	/// <summary>Unconditional branch to switch section. <c>goto case target;</c></summary>
-	public sealed partial class GotoCase : SimpleInstruction
-	{
-		public override StackType ResultType { get { return StackType.Void; } }
-		protected override InstructionFlags ComputeFlags()
-		{
-			return InstructionFlags.EndPointUnreachable | InstructionFlags.MayBranch;
-		}
-		public override InstructionFlags DirectFlags {
-			get {
-				return InstructionFlags.EndPointUnreachable | InstructionFlags.MayBranch;
-			}
-		}
-		public override void AcceptVisitor(ILVisitor visitor)
-		{
-			visitor.VisitGotoCase(this);
-		}
-		public override T AcceptVisitor<T>(ILVisitor<T> visitor)
-		{
-			return visitor.VisitGotoCase(this);
-		}
-		public override T AcceptVisitor<C, T>(ILVisitor<C, T> visitor, C context)
-		{
-			return visitor.VisitGotoCase(this, context);
-		}
-		protected internal override bool PerformMatch(ILInstruction other, ref Patterns.Match match)
-		{
-			var o = other as GotoCase;
-			return o != null && this.TargetSection == o.TargetSection;
-		}
-	}
-}
-namespace ICSharpCode.Decompiler.IL
 {
 	/// <summary>Try-catch statement.</summary>
 	public sealed partial class TryCatch : TryInstruction
@@ -4573,10 +4537,6 @@ namespace ICSharpCode.Decompiler.IL
 		{
 			Default(inst);
 		}
-		protected internal virtual void VisitGotoCase(GotoCase inst)
-		{
-			Default(inst);
-		}
 		protected internal virtual void VisitTryCatch(TryCatch inst)
 		{
 			Default(inst);
@@ -4867,10 +4827,6 @@ namespace ICSharpCode.Decompiler.IL
 		{
 			return Default(inst);
 		}
-		protected internal virtual T VisitGotoCase(GotoCase inst)
-		{
-			return Default(inst);
-		}
 		protected internal virtual T VisitTryCatch(TryCatch inst)
 		{
 			return Default(inst);
@@ -5161,10 +5117,6 @@ namespace ICSharpCode.Decompiler.IL
 		{
 			return Default(inst, context);
 		}
-		protected internal virtual T VisitGotoCase(GotoCase inst, C context)
-		{
-			return Default(inst, context);
-		}
 		protected internal virtual T VisitTryCatch(TryCatch inst, C context)
 		{
 			return Default(inst, context);
@@ -5399,7 +5351,6 @@ namespace ICSharpCode.Decompiler.IL
 			"if.notnull",
 			"switch",
 			"switch.section",
-			"goto.case",
 			"try.catch",
 			"try.catch.handler",
 			"try.finally",

ICSharpCode.Decompiler/IL/Instructions.tt

@@ -98,9 +98,6 @@
 			CustomClassName("SwitchSection"), CustomChildren(new [] { new ChildInfo("body") }),
 			CustomConstructor, CustomComputeFlags, CustomWriteTo, ResultType("Void"),
 			MatchCondition("this.Labels.SetEquals(o.Labels) && this.HasNullLabel == o.HasNullLabel")),
-		new OpCode("goto.case", "Unconditional branch to switch section. <c>goto case target;</c>",
-			CustomClassName("GotoCase"), NoArguments, CustomConstructor, UnconditionalBranch, MayBranch,
-			MatchCondition("this.TargetSection == o.TargetSection")),
 		new OpCode("try.catch", "Try-catch statement.",
 			BaseClass("TryInstruction"), CustomConstructor, CustomComputeFlags, CustomWriteTo,
 			MatchCondition("TryBlock.PerformMatch(o.TryBlock, ref match)"),

ICSharpCode.Decompiler/IL/Instructions/Block.cs

@@ -108,6 +108,9 @@ namespace ICSharpCode.Decompiler.IL
 				// only the last instruction may have an unreachable endpoint
 				Debug.Assert(!Instructions[i].HasFlag(InstructionFlags.EndPointUnreachable));
 			}
+			if (this.Type == BlockType.ControlFlow) {
+				Debug.Assert(finalInstruction.OpCode == OpCode.Nop);
+			}
 		}
 		
 		public override StackType ResultType {

ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs

@@ -154,7 +154,7 @@ namespace ICSharpCode.Decompiler.IL
 			Debug.Assert(EntryPoint == Blocks[0]);
 			Debug.Assert(!IsConnected || EntryPoint.IncomingEdgeCount >= 1);
 			Debug.Assert(Blocks.All(b => b.HasFlag(InstructionFlags.EndPointUnreachable)));
-			Debug.Assert(Blocks.All(b => b.FinalInstruction.OpCode == OpCode.Nop));
+			Debug.Assert(Blocks.All(b => b.Type == BlockType.ControlFlow)); // this also implies that the blocks don't use FinalInstruction
 		}
 		
 		protected override InstructionFlags ComputeFlags()

ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs

@@ -196,37 +196,4 @@ namespace ICSharpCode.Decompiler.IL
 			body.WriteTo(output, options);
 		}
 	}
-
-	/// <summary>
-	/// Unconditional branch to switch section. <c>goto case target;</c>/<c>goto default;</c>
-	/// </summary>
-	/// <remarks>
-	/// Like normal branches, can trigger finally blocks.
-	/// </remarks>
-	partial class GotoCase // : IBranchOrLeaveInstruction
-	{
-		public SwitchSection TargetSection { get; }
-
-		public GotoCase(SwitchSection target) : base(OpCode.GotoCase)
-		{
-			this.TargetSection = target ?? throw new ArgumentNullException(nameof(target));
-		}
-
-		internal override void CheckInvariant(ILPhase phase)
-		{
-			base.CheckInvariant(phase);
-			var parentSwitch = this.Ancestors.OfType<SwitchInstruction>().First();
-			Debug.Assert(parentSwitch == TargetSection.Parent);
-		}
-
-		public override void WriteTo(ITextOutput output, ILAstWritingOptions options)
-		{
-			output.Write("goto.case ");
-			if (TargetSection.HasNullLabel) {
-				output.WriteReference("null", TargetSection, isLocal: true);
-			} else {
-				output.WriteReference(TargetSection.Labels.Values.First().ToString(), TargetSection, isLocal: true);
-			}
-		}
-	}
 }