ConditionDetection: move blocks into switch sections

ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@@ -52,8 +52,10 @@ namespace ICSharpCode.Decompiler.CSharp
 		public static List<IILTransform> GetILTransforms()
 		{
 			return new List<IILTransform> {
-				new SplitVariables(),
 				new ControlFlowSimplification(),
+				// Run SplitVariables only after ControlFlowSimplification duplicates return blocks,
+				// so that the return variable is split and can be inlined.
+				new SplitVariables(),
 				new ILInlining(),
 				new DetectPinnedRegions(), // must run after inlining but before non-critical control flow transforms
 				new SwitchDetection(),

ICSharpCode.Decompiler/CSharp/StatementBuilder.cs

@@ -93,7 +93,14 @@ namespace ICSharpCode.Decompiler.CSharp
 			foreach (var section in inst.Sections) {
 				var astSection = new ICSharpCode.NRefactory.CSharp.SwitchSection();
 				astSection.CaseLabels.AddRange(section.Labels.Values.Select(i => CreateTypedCaseLabel(i, value.Type)));
-				astSection.Statements.Add(Convert(section.Body));
+				ConvertSwitchSectionBody(astSection, section.Body);
+				stmt.SwitchSections.Add(astSection);
+			}
+			
+			if (inst.DefaultBody.OpCode != OpCode.Nop) {
+				var astSection = new ICSharpCode.NRefactory.CSharp.SwitchSection();
+				astSection.CaseLabels.Add(new CaseLabel());
+				ConvertSwitchSectionBody(astSection, inst.DefaultBody);
 				stmt.SwitchSections.Add(astSection);
 			}
 
@@ -101,6 +108,21 @@ namespace ICSharpCode.Decompiler.CSharp
 			return stmt;
 		}
 
+		private void ConvertSwitchSectionBody(NRefactory.CSharp.SwitchSection astSection, ILInstruction bodyInst)
+		{
+			var body = Convert(bodyInst);
+			astSection.Statements.Add(body);
+			if (!bodyInst.HasFlag(InstructionFlags.EndPointUnreachable)) {
+				// we need to insert 'break;'
+				BlockStatement block = body as BlockStatement;
+				if (block != null) {
+					block.Add(new BreakStatement());
+				} else {
+					astSection.Statements.Add(new BreakStatement());
+				}
+			}
+		}
+		
 		/// <summary>Target block that a 'continue;' statement would jump to</summary>
 		Block continueTarget;
 		/// <summary>Number of ContinueStatements that were created for the current continueTarget</summary>

ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs

@@ -65,10 +65,11 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 		{
 			public readonly IType Type;
 			public readonly string Name;
+
 			/// <summary>
 			/// Whether the variable needs to be default-initialized.
 			/// </summary>
-			public readonly bool DefaultInitialization;
+			public bool DefaultInitialization;
 			
 			/// <summary>
 			/// Integer value that can be used to compare to VariableToDeclare instances
@@ -237,6 +238,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 						} else {
 							v.InsertionPoint = point2;
 						}
+						v.DefaultInitialization |= prev.DefaultInitialization;
 						// I think we don't need to re-check the dict entries that we already checked earlier,
 						// because the new v.InsertionPoint only collides with another point x if either
 						// the old v.InsertionPoint or the old prev.InsertionPoint already collided with x.

ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs

@@ -569,8 +569,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis
 			DebugStartPoint(inst);
 			inst.Value.AcceptVisitor(this);
 			State beforeSections = state.Clone();
-			// It's possible for none of the cases to match,
-			// so afterSections is reachable without executing any section
+			inst.DefaultBody.AcceptVisitor(this);
 			State afterSections = state.Clone();
 			foreach (var section in inst.Sections) {
 				state.ReplaceWith(beforeSections);

ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs

@@ -78,60 +78,11 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			// usually an IfInstruction with a branch:
 			IfInstruction ifInst = block.Instructions.SecondToLastOrDefault() 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;"
-					// -> "if (...) { targetBlock } exitInst;"
-					var targetBlock = ((Branch)ifInst.TrueInst).TargetBlock;
-					// The targetBlock was already processed, we can embed it into the if statement:
-					ifInst.TrueInst = targetBlock;
-					trueExitInst = targetBlock.Instructions.LastOrDefault();
-					if (CompatibleExitInstruction(exitInst, trueExitInst)) {
-						// "if (...) { ...; goto exitPoint } goto exitPoint;"
-						// -> "if (...) { ... } goto exitPoint;"
-						targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1);
-						trueExitInst = null;
-					}
-				} else {
-					trueExitInst = ifInst.TrueInst;
-				}
-				if (IsUsableBranchToChild(cfgNode, exitInst)) {
-					var targetBlock = ((Branch)exitInst).TargetBlock;
-					var falseExitInst = targetBlock.Instructions.LastOrDefault();
-					if (CompatibleExitInstruction(trueExitInst, falseExitInst)) {
-						// if (...) { ...; goto exitPoint; } goto nextBlock; nextBlock: ...; goto exitPoint;
-						// -> if (...) { ... } else { ... } goto exitPoint;
-						targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1);
-						ifInst.FalseInst = targetBlock;
-						exitInst = block.Instructions[block.Instructions.Count - 1] = falseExitInst;
-						Block trueBlock = ifInst.TrueInst as Block;
-						if (trueBlock != null) {
-							Debug.Assert(trueExitInst == trueBlock.Instructions.Last());
-							trueBlock.Instructions.RemoveAt(trueBlock.Instructions.Count - 1);
-						} else {
-							Debug.Assert(trueExitInst == ifInst.TrueInst);
-							ifInst.TrueInst = new Nop { ILRange = ifInst.TrueInst.ILRange };
-						}
-					}
-				}
-				if (ifInst.FalseInst.OpCode != OpCode.Nop && ifInst.FalseInst.ILRange.Start < ifInst.TrueInst.ILRange.Start
-				    || ifInst.TrueInst.OpCode == OpCode.Nop)
-				{
-					// swap true and false branches of if, to bring them in the same order as the IL code
-					var oldTrue = ifInst.TrueInst;
-					ifInst.TrueInst = ifInst.FalseInst;
-					ifInst.FalseInst = oldTrue;
-					ifInst.Condition = new LogicNot(ifInst.Condition);
+				HandleIfInstruction(cfgNode, block, ifInst, ref exitInst);
+			} else {
+				SwitchInstruction switchInst = block.Instructions.SecondToLastOrDefault() as SwitchInstruction;
+				if (switchInst != null) {
+					HandleSwitchInstruction(cfgNode, block, switchInst, ref exitInst);
 				}
 			}
 			if (IsUsableBranchToChild(cfgNode, exitInst)) {
@@ -145,6 +96,64 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			}
 		}
 
+		private void HandleIfInstruction(ControlFlowNode cfgNode, Block block, IfInstruction ifInst, ref ILInstruction exitInst)
+		{
+			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;"
+				// -> "if (...) { targetBlock } exitInst;"
+				var targetBlock = ((Branch)ifInst.TrueInst).TargetBlock;
+				// The targetBlock was already processed, we can embed it into the if statement:
+				ifInst.TrueInst = targetBlock;
+				trueExitInst = targetBlock.Instructions.LastOrDefault();
+				if (CompatibleExitInstruction(exitInst, trueExitInst)) {
+					// "if (...) { ...; goto exitPoint } goto exitPoint;"
+					// -> "if (...) { ... } goto exitPoint;"
+					targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1);
+					trueExitInst = null;
+				}
+			} else {
+				trueExitInst = ifInst.TrueInst;
+			}
+			if (IsUsableBranchToChild(cfgNode, exitInst)) {
+				var targetBlock = ((Branch)exitInst).TargetBlock;
+				var falseExitInst = targetBlock.Instructions.LastOrDefault();
+				if (CompatibleExitInstruction(trueExitInst, falseExitInst)) {
+					// if (...) { ...; goto exitPoint; } goto nextBlock; nextBlock: ...; goto exitPoint;
+					// -> if (...) { ... } else { ... } goto exitPoint;
+					targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1);
+					ifInst.FalseInst = targetBlock;
+					exitInst = block.Instructions[block.Instructions.Count - 1] = falseExitInst;
+					Block trueBlock = ifInst.TrueInst as Block;
+					if (trueBlock != null) {
+						Debug.Assert(trueExitInst == trueBlock.Instructions.Last());
+						trueBlock.Instructions.RemoveAt(trueBlock.Instructions.Count - 1);
+					} else {
+						Debug.Assert(trueExitInst == ifInst.TrueInst);
+						ifInst.TrueInst = new Nop { ILRange = ifInst.TrueInst.ILRange };
+					}
+				}
+			}
+			if (ifInst.FalseInst.OpCode != OpCode.Nop && ifInst.FalseInst.ILRange.Start < ifInst.TrueInst.ILRange.Start
+				|| ifInst.TrueInst.OpCode == OpCode.Nop) {
+				// swap true and false branches of if, to bring them in the same order as the IL code
+				var oldTrue = ifInst.TrueInst;
+				ifInst.TrueInst = ifInst.FalseInst;
+				ifInst.FalseInst = oldTrue;
+				ifInst.Condition = new LogicNot(ifInst.Condition);
+			}
+		}
+
 		bool IsBranchToLaterTarget(ILInstruction inst1, ILInstruction inst2)
 		{
 			Block block1, block2;
@@ -185,5 +194,38 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 					return false;
 			}
 		}
+
+		private void HandleSwitchInstruction(ControlFlowNode cfgNode, Block block, SwitchInstruction sw, ref ILInstruction exitInst)
+		{
+			Debug.Assert(sw.DefaultBody is Nop);
+			foreach (var section in sw.Sections) {
+				if (IsUsableBranchToChild(cfgNode, section.Body)) {
+					// case ...: goto targetBlock;
+					var targetBlock = ((Branch)section.Body).TargetBlock;
+					section.Body = targetBlock;
+				}
+			}
+			if (IsUsableBranchToChild(cfgNode, exitInst)) {
+				// switch(...){} goto targetBlock;
+				// ---> switch(..) { default: { targetBlock } }
+				var targetBlock = ((Branch)exitInst).TargetBlock;
+				sw.DefaultBody = targetBlock;
+				if (targetBlock.Instructions.Last().OpCode == OpCode.Branch || targetBlock.Instructions.Last().OpCode == OpCode.Leave) {
+					exitInst = block.Instructions[block.Instructions.Count - 1] = targetBlock.Instructions.Last();
+					targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1);
+				} else {
+					exitInst = null;
+					block.Instructions.RemoveAt(block.Instructions.Count - 1);
+				}
+			}
+			// Remove compatible exitInsts from switch sections:
+			foreach (var section in sw.Sections) {
+				Block sectionBlock = section.Body as Block;
+				if (sectionBlock != null && CompatibleExitInstruction(exitInst, sectionBlock.Instructions.Last())) {
+					sectionBlock.Instructions.RemoveAt(sectionBlock.Instructions.Count - 1);
+				}
+			}
+			sw.Sections.ReplaceList(sw.Sections.OrderBy(s => s.Body.ILRange.Start));
+		}
 	}
 }

ICSharpCode.Decompiler/IL/ControlFlow/SwitchAnalysis.cs

@@ -131,6 +131,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 
 		private bool AnalyzeSwitch(SwitchInstruction inst, LongSet inputValues, out LongSet anyMatchValues)
 		{
+			Debug.Assert(inst.DefaultBody is Nop);
 			anyMatchValues = LongSet.Empty;
 			long offset;
 			if (MatchSwitchVar(inst.Value)) {

ICSharpCode.Decompiler/IL/Instructions.cs

@@ -1103,7 +1103,7 @@ namespace ICSharpCode.Decompiler.IL
 		protected internal override bool PerformMatch(ILInstruction other, ref Patterns.Match match)
 		{
 			var o = other as SwitchInstruction;
-			return o != null && Value.PerformMatch(o.Value, ref match) && Patterns.ListMatch.DoMatch(this.Sections, o.Sections, ref match);
+			return o != null && Value.PerformMatch(o.Value, ref match) && DefaultBody.PerformMatch(o.DefaultBody, ref match) && Patterns.ListMatch.DoMatch(this.Sections, o.Sections, ref match);
 		}
 	}
 

ICSharpCode.Decompiler/IL/Instructions.tt

@@ -86,7 +86,7 @@
 			}), CustomConstructor, CustomComputeFlags, CustomWriteTo),
 		new OpCode("switch", "Switch statement",
 			CustomClassName("SwitchInstruction"), CustomConstructor, CustomComputeFlags, CustomWriteTo, ResultType("Void"),
-			MatchCondition("Value.PerformMatch(o.Value, ref match) && Patterns.ListMatch.DoMatch(this.Sections, o.Sections, ref match)")),
+			MatchCondition("Value.PerformMatch(o.Value, ref match) && DefaultBody.PerformMatch(o.DefaultBody, ref match) && Patterns.ListMatch.DoMatch(this.Sections, o.Sections, ref match)")),
 		new OpCode("switch.section", "Switch section within a switch statement",
 			CustomClassName("SwitchSection"), CustomChildren(new [] { new ChildInfo("body") }),
 			CustomConstructor, CustomComputeFlags, CustomWriteTo, ResultType("Void"),

ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs

@@ -33,13 +33,15 @@ namespace ICSharpCode.Decompiler.IL
 	partial class SwitchInstruction
 	{
 		public static readonly SlotInfo ValueSlot = new SlotInfo("Value", canInlineInto: true);
+		public static readonly SlotInfo DefaultBodySlot = new SlotInfo("DefaultBody");
 		public static readonly SlotInfo SectionSlot = new SlotInfo("Section", isCollection: true);
 		
 		public SwitchInstruction(ILInstruction value)
 			: base(OpCode.SwitchInstruction)
 		{
 			this.Value = value;
-			this.Sections = new InstructionCollection<SwitchSection>(this, 1);
+			this.DefaultBody = new Nop();
+			this.Sections = new InstructionCollection<SwitchSection>(this, 2);
 		}
 		
 		ILInstruction value;
@@ -51,16 +53,25 @@ namespace ICSharpCode.Decompiler.IL
 			}
 		}
 		
+		ILInstruction defaultBody;
+
+		public ILInstruction DefaultBody {
+			get { return this.defaultBody; }
+			set {
+				ValidateChild(value);
+				SetChildInstruction(ref this.defaultBody, value, 1);
+			}
+		}
+
 		public readonly InstructionCollection<SwitchSection> Sections;
 
 		protected override InstructionFlags ComputeFlags()
 		{
-			var sectionFlags = InstructionFlags.ControlFlow;
-			// note: the initial sectionFlags also represent the implicit empty 'default' case
+			var sectionFlags = defaultBody.Flags;
 			foreach (var section in Sections) {
 				sectionFlags = SemanticHelper.CombineBranches(sectionFlags, section.Flags);
 			}
-			return value.Flags | sectionFlags;
+			return value.Flags | InstructionFlags.ControlFlow | sectionFlags;
 		}
 		
 		public override InstructionFlags DirectFlags {
@@ -77,6 +88,9 @@ namespace ICSharpCode.Decompiler.IL
 			output.MarkFoldStart("{...}");
 			output.WriteLine("{");
 			output.Indent();
+			output.Write("default: ");
+			defaultBody.WriteTo(output);
+			output.WriteLine();
 			foreach (var section in this.Sections) {
 				section.WriteTo(output);
 				output.WriteLine();
@@ -88,28 +102,34 @@ namespace ICSharpCode.Decompiler.IL
 		
 		protected override int GetChildCount()
 		{
-			return 1 + Sections.Count;
+			return 2 + Sections.Count;
 		}
 		
 		protected override ILInstruction GetChild(int index)
 		{
 			if (index == 0)
 				return value;
-			return Sections[index - 1];
+			else if (index == 1)
+				return defaultBody;
+			return Sections[index - 2];
 		}
 		
 		protected override void SetChild(int index, ILInstruction value)
 		{
 			if (index == 0)
 				Value = value;
+			else if (index == 1)
+				DefaultBody = value;
 			else
-				Sections[index - 1] = (SwitchSection)value;
+				Sections[index - 2] = (SwitchSection)value;
 		}
 		
 		protected override SlotInfo GetChildSlot(int index)
 		{
 			if (index == 0)
 				return ValueSlot;
+			else if (index == 1)
+				return DefaultBodySlot;
 			return SectionSlot;
 		}
 		
@@ -118,6 +138,7 @@ namespace ICSharpCode.Decompiler.IL
 			var clone = new SwitchInstruction(value.Clone());
 			clone.ILRange = this.ILRange;
 			clone.Value = value.Clone();
+			this.DefaultBody = defaultBody.Clone();
 			clone.Sections.AddRange(this.Sections.Select(h => (SwitchSection)h.Clone()));
 			return clone;
 		}