Allow detecting exit points across multiple levels of containers.

ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.cs

@@ -1009,5 +1009,26 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 			}
 			Console.WriteLine("end");
 		}
+
+		public void ForEachInSwitch(int i, IEnumerable<string> args)
+		{
+			switch (i)
+			{
+
+				case 1:
+					Console.WriteLine("one");
+					break;
+				case 2:
+				{
+					foreach (string arg in args)
+					{
+						Console.WriteLine(arg);
+					}
+					break;
+				}
+				default:
+					throw new NotImplementedException();
+			}
+		}
 	}
 }

ICSharpCode.Decompiler/IL/ControlFlow/ExitPoints.cs

@@ -16,8 +16,11 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 // DEALINGS IN THE SOFTWARE.
 
+#nullable enable
+
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Linq;
 using System.Threading;
 
 using ICSharpCode.Decompiler.IL.Transforms;
@@ -59,10 +62,10 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 		/// </summary>
 		internal static ILInstruction GetExit(ILInstruction inst)
 		{
-			SlotInfo slot = inst.SlotInfo;
+			SlotInfo? slot = inst.SlotInfo;
 			if (slot == Block.InstructionSlot)
 			{
-				Block block = (Block)inst.Parent;
+				Block block = (Block)inst.Parent!;
 				return block.Instructions.ElementAtOrDefault(inst.ChildIndex + 1) ?? ExitNotYetDetermined;
 			}
 			else if (slot == TryInstruction.TryBlockSlot
@@ -72,7 +75,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			  || slot == UsingInstruction.BodySlot
 			  || slot == LockInstruction.BodySlot)
 			{
-				return GetExit(inst.Parent);
+				return GetExit(inst.Parent!);
 			}
 			return NoExit;
 		}
@@ -100,30 +103,53 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			}
 		}
 
-		CancellationToken cancellationToken;
-		BlockContainer currentContainer;
+		class ContainerContext
+		{
+			public readonly BlockContainer Container;
 
-		/// <summary>
-		/// The instruction that will be executed next after leaving the currentContainer.
-		/// <c>ExitNotYetDetermined</c> means the container is last in its parent block, and thus does not
-		/// yet have any leave instructions. This means we can move any exit instruction of
-		/// our choice our of the container and replace it with a leave instruction.
-		/// </summary>
-		ILInstruction currentExit;
+			/// <summary>
+			/// The instruction that will be executed next after leaving the Container.
+			/// <c>ExitNotYetDetermined</c> means the container is last in its parent block, and thus does not
+			/// yet have any leave instructions. This means we can move any exit instruction of
+			/// our choice our of the container and replace it with a leave instruction.
+			/// </summary>
+			public readonly ILInstruction CurrentExit;
 
-		/// <summary>
-		/// If <c>currentExit==ExitNotYetDetermined</c>, holds the list of potential exit instructions.
-		/// After the currentContainer was visited completely, one of these will be selected as exit instruction.
-		/// </summary>
-		List<ILInstruction> potentialExits;
+			/// <summary>
+			/// If <c>currentExit==ExitNotYetDetermined</c>, holds the list of potential exit instructions.
+			/// After the currentContainer was visited completely, one of these will be selected as exit instruction.
+			/// </summary>
+			public readonly List<ILInstruction>? PotentialExits = null;
+
+			public ContainerContext(BlockContainer container, ILInstruction currentExit)
+			{
+				this.Container = container;
+				this.CurrentExit = currentExit;
+				this.PotentialExits = (currentExit == ExitNotYetDetermined ? new List<ILInstruction>() : null);
+			}
+
+			public void HandleExit(ILInstruction inst)
+			{
+				if (this.CurrentExit == ExitNotYetDetermined && this.Container.LeaveCount == 0)
+				{
+					this.PotentialExits!.Add(inst);
+				}
+				else if (CompatibleExitInstruction(inst, this.CurrentExit))
+				{
+					inst.ReplaceWith(new Leave(this.Container).WithILRange(inst));
+				}
+			}
+		}
 
+		CancellationToken cancellationToken;
 		readonly List<Block> blocksPotentiallyMadeUnreachable = new List<Block>();
+		readonly Stack<ContainerContext> containerStack = new Stack<ContainerContext>();
 
 		public void Run(ILFunction function, ILTransformContext context)
 		{
 			cancellationToken = context.CancellationToken;
-			currentExit = NoExit;
 			blocksPotentiallyMadeUnreachable.Clear();
+			containerStack.Clear();
 			function.AcceptVisitor(this);
 			// It's possible that there are unreachable code blocks which we only
 			// detect as such during exit point detection.
@@ -136,6 +162,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				}
 			}
 			blocksPotentiallyMadeUnreachable.Clear();
+			containerStack.Clear();
 		}
 
 		static bool IsInfiniteLoop(Block block)
@@ -153,30 +180,26 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 
 		protected internal override void VisitBlockContainer(BlockContainer container)
 		{
-			var oldExit = currentExit;
-			var oldContainer = currentContainer;
-			var oldPotentialExits = potentialExits;
 			var thisExit = GetExit(container);
-			currentExit = thisExit;
-			currentContainer = container;
-			potentialExits = (thisExit == ExitNotYetDetermined ? new List<ILInstruction>() : null);
+			var stackEntry = new ContainerContext(container, thisExit);
+			containerStack.Push(stackEntry);
 			base.VisitBlockContainer(container);
-			if (thisExit == ExitNotYetDetermined && potentialExits.Count > 0)
+			if (stackEntry.PotentialExits?.Any(i => i.IsConnected) ?? false)
 			{
 				// This transform determined an exit point.
-				currentExit = ChooseExit(potentialExits);
-				foreach (var exit in potentialExits)
+				var newExit = ChooseExit(stackEntry.PotentialExits.Where(i => i.IsConnected));
+				Debug.Assert(!newExit.MatchLeave(container));
+				foreach (var exit in stackEntry.PotentialExits)
 				{
-					if (CompatibleExitInstruction(currentExit, exit))
+					if (exit.IsConnected && CompatibleExitInstruction(newExit, exit))
 					{
-						exit.ReplaceWith(new Leave(currentContainer).WithILRange(exit));
+						exit.ReplaceWith(new Leave(container).WithILRange(exit));
 					}
 				}
-				Debug.Assert(!currentExit.MatchLeave(currentContainer));
 				ILInstruction inst = container;
 				// traverse up to the block (we'll always find one because GetExit
 				// only returns ExitNotYetDetermined if there's a block)
-				while (inst.Parent.OpCode != OpCode.Block)
+				while (inst.Parent!.OpCode != OpCode.Block)
 					inst = inst.Parent;
 				Block block = (Block)inst.Parent;
 				if (block.HasFlag(InstructionFlags.EndPointUnreachable))
@@ -185,33 +208,32 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 					// we still have an unreachable endpoint.
 					// The appended currentExit instruction would not be reachable!
 					// This happens in test case ExceptionHandling.ThrowInFinally()
-					if (currentExit is Branch b)
+					if (newExit is Branch b)
 					{
 						blocksPotentiallyMadeUnreachable.Add(b.TargetBlock);
 					}
 				}
 				else
 				{
-					block.Instructions.Add(currentExit);
+					block.Instructions.Add(newExit);
 				}
 			}
-			else
+			if (containerStack.Pop() != stackEntry)
 			{
-				Debug.Assert(thisExit == currentExit);
+				Debug.Fail("containerStack got imbalanced");
 			}
-			currentExit = oldExit;
-			currentContainer = oldContainer;
-			potentialExits = oldPotentialExits;
 		}
 
-		static ILInstruction ChooseExit(List<ILInstruction> potentialExits)
+		static ILInstruction ChooseExit(IEnumerable<ILInstruction> potentialExits)
 		{
-			ILInstruction first = potentialExits[0];
+			using var enumerator = potentialExits.GetEnumerator();
+			enumerator.MoveNext();
+			ILInstruction first = enumerator.Current;
 			if (first is Leave { IsLeavingFunction: true })
 			{
-				for (int i = 1; i < potentialExits.Count; i++)
+				while (enumerator.MoveNext())
 				{
-					var exit = potentialExits[i];
+					var exit = enumerator.Current;
 					if (!(exit is Leave { IsLeavingFunction: true }))
 						return exit;
 				}
@@ -229,43 +251,13 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			}
 		}
 
-		void HandleExit(ILInstruction inst)
-		{
-			if (currentExit == ExitNotYetDetermined && CanIntroduceAsExit(inst))
-			{
-				potentialExits.Add(inst);
-			}
-			else if (CompatibleExitInstruction(inst, currentExit))
-			{
-				inst.ReplaceWith(new Leave(currentContainer).WithILRange(inst));
-			}
-		}
-
-		private bool CanIntroduceAsExit(ILInstruction inst)
-		{
-			if (currentContainer.LeaveCount > 0)
-			{
-				// if we're re-running on a block container that already has an exit,
-				// we can't introduce any additional exits
-				return false;
-			}
-			if (inst is Leave { IsLeavingFunction: true })
-			{
-				// Only convert 'return;' to an exit in a context where we can turn it into 'break;'.
-				// In other contexts we risk turning it into 'goto'.
-				return currentContainer.Kind != ContainerKind.Normal;
-			}
-			else
-			{
-				return true;
-			}
-		}
-
 		protected internal override void VisitBranch(Branch inst)
 		{
-			if (!inst.TargetBlock.IsDescendantOf(currentContainer))
+			foreach (var entry in containerStack)
 			{
-				HandleExit(inst);
+				if (inst.TargetBlock.IsDescendantOf(entry.Container))
+					break;
+				entry.HandleExit(inst);
 			}
 		}
 
@@ -274,7 +266,32 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			base.VisitLeave(inst);
 			if (!inst.Value.MatchNop())
 				return;
-			HandleExit(inst);
+			foreach (var entry in containerStack)
+			{
+				if (inst.TargetContainer == entry.Container)
+					break;
+				if (inst.IsLeavingFunction || inst.TargetContainer.Kind != ContainerKind.Normal)
+				{
+					if (entry.Container.Kind == ContainerKind.Normal)
+					{
+						// Don't transform a `return`/`break` into a leave for try-block containers (or similar).
+						// It's possible that those can be turned into fallthrough later, but
+						// it might not work out and then we would be left with a `goto`.
+						// But continue searching the container stack, it might be possible to
+						// turn the `return` into a `break` instead.
+					}
+					else
+					{
+						// return; could turn to break;
+						entry.HandleExit(inst);
+						break; // but only for the innermost loop/switch
+					}
+				}
+				else
+				{
+					entry.HandleExit(inst);
+				}
+			}
 		}
 	}
 }