Fix 'yield break;' in try-catch blocks.

ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@@ -70,7 +70,7 @@ namespace ICSharpCode.Decompiler.CSharp
 				new ILInlining(),
 				new DetectPinnedRegions(), // must run after inlining but before non-critical control flow transforms
 				new YieldReturnDecompiler(), // must run after inlining but before loop detection
-				new DetectExitPoints(),
+				new DetectExitPoints(canIntroduceExitForReturn: false),
 				new BlockILTransform {
 					PostOrderTransforms = {
 						new ExpressionTransforms() // for RemoveDeadVariableInit
@@ -109,6 +109,7 @@ namespace ICSharpCode.Decompiler.CSharp
 						)
 					}
 				},
+				//new DetectExitPoints(canIntroduceExitForReturn: true),
 				new DelegateConstruction(),
 			};
 		}

ICSharpCode.Decompiler/IL/ControlFlow/ExitPoints.cs

@@ -49,7 +49,14 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 	{
 		static readonly Nop ExitNotYetDetermined = new Nop();
 		static readonly Nop NoExit = new Nop();
-		
+
+		bool canIntroduceExitForReturn;
+
+		public DetectExitPoints(bool canIntroduceExitForReturn)
+		{
+			this.canIntroduceExitForReturn = canIntroduceExitForReturn;
+		}
+
 		/// <summary>
 		/// Gets the next instruction after <paramref name="inst"/> is executed.
 		/// Returns NoExit when the next instruction cannot be identified;
@@ -150,7 +157,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 
 		void HandleExit(ILInstruction inst)
 		{
-			if (currentExit == ExitNotYetDetermined && !(inst is Leave l && l.IsLeavingFunction)) {
+			if (currentExit == ExitNotYetDetermined && CanIntroduceAsExit(inst)) {
 				currentExit = inst;
 				inst.ReplaceWith(new Leave(currentContainer) { ILRange = inst.ILRange });
 			} else if (CompatibleExitInstruction(inst, currentExit)) {
@@ -158,6 +165,15 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			}
 		}
 
+		private bool CanIntroduceAsExit(ILInstruction inst)
+		{
+			if (inst is Leave l && l.IsLeavingFunction) {
+				return canIntroduceExitForReturn;
+			} else {
+				return true;
+			}
+		}
+
 		protected internal override void VisitBranch(Branch inst)
 		{
 			if (!inst.TargetBlock.IsDescendantOf(currentContainer)) {

ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs

@@ -84,12 +84,10 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 		/// </summary>
 		readonly Dictionary<IMethod, (int? outerState, BlockContainer body)> decompiledFinallyMethods = new Dictionary<IMethod, (int? outerState, BlockContainer body)>();
 
-		/*
 		/// <summary>
-		/// List of blocks that change the iterator state on block entry.
+		/// Temporary stores for 'yield break'.
 		/// </summary>
-		readonly List<(int state, Block block)> stateChanges = new List<(int state, Block block)>();
+		readonly List<StLoc> returnStores = new List<StLoc>();
-		*/
 
 		#region Run() method
 		public void Run(ILFunction function, ILTransformContext context)
@@ -105,6 +103,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			this.fieldToParameterMap.Clear();
 			this.finallyMethodToStateRange = null;
 			this.decompiledFinallyMethods.Clear();
+			this.returnStores.Clear();
 			if (!MatchEnumeratorCreationPattern(function))
 				return;
 			BlockContainer newBody;
@@ -138,6 +137,15 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			context.Step("Translate fields to local accesses", function);
 			TranslateFieldsToLocalAccess(function, function, fieldToParameterMap);
 
+			if (returnStores.Count > 0) {
+				context.Step("Remove temporaries", function);
+				foreach (var store in returnStores) {
+					if (store.Variable.LoadCount == 0 && store.Variable.AddressCount == 0 && store.Parent is Block block) {
+						block.Instructions.Remove(store);
+					}
+				}
+			}
+
 			// Re-run control flow simplification over the newly constructed set of gotos,
 			// and inlining because TranslateFieldsToLocalAccess() might have opened up new inlining opportunities.
 			function.RunTransforms(CSharpDecompiler.EarlyILTransforms(), context);
@@ -440,7 +448,6 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				newBody.Blocks.Add(new Block { ILRange = oldBody.Blocks[blockIndex].ILRange });
 			}
 			// convert contents of blocks
-			var ssaDefs = new Dictionary<ILVariable, ILInstruction>();
 			
 			for (int i = 0; i < oldBody.Blocks.Count; i++) {
 				var oldBlock = oldBody.Blocks[i];
@@ -472,23 +479,10 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 						// Break up the basic block on a call to a finally method
 						// (this allows us to consider each block individually for try-finally reconstruction)
 						newBlock = SplitBlock(newBlock, oldInst);
-					} else if (oldInst.MatchReturn(out value)) {
-						if (value.MatchLdLoc(out var v)) {
-							ssaDefs.TryGetValue(v, out value);
-						}
-						if (value != null && value.MatchLdcI4(0)) {
-							// yield break
-							newBlock.Instructions.Add(new Leave(newBody) { ILRange = oldInst.ILRange });
-						} else {
-							newBlock.Instructions.Add(new InvalidBranch("Unexpected return in MoveNext()"));
-						}
-						break; // we're done with this basic block
-					} else if (oldInst.MatchStLoc(out var v, out value) && v.IsSingleDefinition) {
-						ssaDefs.Add(v, value);
 					}
 					// copy over the instruction to the new block
-					UpdateBranchTargets(oldInst);
 					newBlock.Instructions.Add(oldInst);
+					UpdateBranchTargets(oldInst);
 				}
 			}
 
@@ -556,6 +550,21 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 							leave.TargetContainer = newBody;
 						}
 						break;
+					case Return ret:
+						ILInstruction value = ret.Value;
+						if (value.MatchLdLoc(out var v) && v.IsSingleDefinition
+							&& v.StoreInstructions.SingleOrDefault() is StLoc stloc)
+						{
+							returnStores.Add(stloc);
+							value = stloc.Value;
+						}
+						if (value.MatchLdcI4(0)) {
+							// yield break
+							ret.ReplaceWith(new Leave(newBody) { ILRange = ret.ILRange });
+						} else {
+							ret.ReplaceWith(new InvalidBranch("Unexpected return in MoveNext()") { ILRange = ret.ILRange });
+						}
+						break;
 				}
 				foreach (var child in inst.Children) {
 					UpdateBranchTargets(child);

ICSharpCode.Decompiler/IL/ILVariable.cs

@@ -19,6 +19,7 @@
 using System;
 using System.Collections.Generic;
 using ICSharpCode.Decompiler.TypeSystem;
+using System.Diagnostics;
 
 namespace ICSharpCode.Decompiler.IL
 {
@@ -125,7 +126,7 @@ namespace ICSharpCode.Decompiler.IL
 		/// </remarks>
 		public int LoadCount => LoadInstructions.Count;
 
-		readonly List<ILoadInstruction> loadInstructions = new List<ILoadInstruction>();
+		readonly List<LdLoc> loadInstructions = new List<LdLoc>();
 
 		/// <summary>
 		/// List of ldloc instructions referencing this variable.
@@ -133,10 +134,11 @@ namespace ICSharpCode.Decompiler.IL
 		/// <remarks>
 		/// This list is automatically updated when adding/removing ldloc instructions from the ILAst.
 		/// </remarks>
-		public IReadOnlyList<ILoadInstruction> LoadInstructions => loadInstructions;
+		public IReadOnlyList<LdLoc> LoadInstructions => loadInstructions;
 		
 		/// <summary>
-		/// Number of store instructions referencing this variable.
+		/// Number of store instructions referencing this variable,
+		/// plus 1 if HasInitialValue.
 		/// 
 		/// Stores are:
 		/// <list type="item">
@@ -161,7 +163,8 @@ namespace ICSharpCode.Decompiler.IL
 		/// <item>stloc</item>
 		/// <item>TryCatchHandler (assigning the exception variable)</item>
 		/// <item>PinnedRegion (assigning the pointer variable)</item>
-		/// <item>initial values (<see cref="HasInitialValue"/>)</item>
+		/// <item>initial values (<see cref="HasInitialValue"/>) -- however, there is no instruction for
+		///       the initial value, so it is not contained in the store list.</item>
 		/// </list>
 		/// </summary>
 		/// <remarks>
@@ -187,30 +190,27 @@ namespace ICSharpCode.Decompiler.IL
 		/// </remarks>
 		public IReadOnlyList<LdLoca> AddressInstructions => addressInstructions;
 
-		internal void AddLoadInstruction(ILoadInstruction inst) => inst.IndexInLoadInstructionList = AddInstruction(loadInstructions, inst);
+		internal void AddLoadInstruction(LdLoc inst) => inst.IndexInLoadInstructionList = AddInstruction(loadInstructions, inst);
 		internal void AddStoreInstruction(IStoreInstruction inst) => inst.IndexInStoreInstructionList = AddInstruction(storeInstructions, inst);
 		internal void AddAddressInstruction(LdLoca inst) => inst.IndexInAddressInstructionList = AddInstruction(addressInstructions, inst);
 
-		internal void RemoveLoadInstruction(ILoadInstruction inst) => RemoveInstruction(loadInstructions, inst.IndexInLoadInstructionList);
+		internal void RemoveLoadInstruction(LdLoc inst) => RemoveInstruction(loadInstructions, inst.IndexInLoadInstructionList, inst);
-		internal void RemoveStoreInstruction(IStoreInstruction inst) => RemoveInstruction(storeInstructions, inst.IndexInStoreInstructionList);
+		internal void RemoveStoreInstruction(IStoreInstruction inst) => RemoveInstruction(storeInstructions, inst.IndexInStoreInstructionList, inst);
-		internal void RemoveAddressInstruction(LdLoca inst) => RemoveInstruction(addressInstructions, inst.IndexInAddressInstructionList);
+		internal void RemoveAddressInstruction(LdLoca inst) => RemoveInstruction(addressInstructions, inst.IndexInAddressInstructionList, inst);
 
-		int AddInstruction<T>(List<T> list, T inst) where T : IInstructionWithVariableOperand
+		int AddInstruction<T>(List<T> list, T inst) where T : class, IInstructionWithVariableOperand
 		{
 			list.Add(inst);
 			return list.Count - 1;
 		}
 
-		void RemoveInstruction<T>(List<T> list, int index) where T : IInstructionWithVariableOperand
+		void RemoveInstruction<T>(List<T> list, int index, T inst) where T : class, IInstructionWithVariableOperand
-	{
+		{
-			if (list.Count > 1) {
+			Debug.Assert(list[index] == inst);
-				int indexToMove = list.Count - 1;
+			int indexToMove = list.Count - 1;
-				list[index] = list[indexToMove];
+			list[index] = list[indexToMove];
-				list[index].IndexInVariableInstructionMapping = index;
+			list[index].IndexInVariableInstructionMapping = index;
-				list.RemoveAt(indexToMove);
+			list.RemoveAt(indexToMove);
-			} else {
-				list.RemoveAt(0);
-			}
 		}
 
 		bool hasInitialValue;
@@ -321,12 +321,12 @@ namespace ICSharpCode.Decompiler.IL
 		int IndexInStoreInstructionList { get; set; }
 	}
 
-	public interface ILoadInstruction : IInstructionWithVariableOperand
+	interface ILoadInstruction : IInstructionWithVariableOperand
 	{
 		int IndexInLoadInstructionList { get; set; }
 	}
 
-	public interface IAddressInstruction : IInstructionWithVariableOperand
+	interface IAddressInstruction : IInstructionWithVariableOperand
 	{
 		int IndexInAddressInstructionList { get; set; }
 	}

ICSharpCode.Decompiler/Tests/ICSharpCode.Decompiler.Tests.csproj

@@ -147,7 +147,7 @@
     <Compile Include="TestCases\Correctness\ValueTypeCall.cs" />
     <Compile Include="CorrectnessTestRunner.cs" />
     <Compile Include="TestCases\Pretty\Loops.cs" />
-    <Compile Include="TestCases\Pretty\YieldReturn.cs" />
+    <Compile Include="TestCases\Pretty\YieldReturnTest.cs" />
     <Compile Include="TestCases\Pretty\CompoundAssignmentTest.cs" />
     <Compile Include="TestCases\Pretty\ExceptionHandling.cs" />
     <Compile Include="TestCases\Pretty\HelloWorld.cs" />

ICSharpCode.Decompiler/Tests/PrettyTestRunner.cs

@@ -104,7 +104,7 @@ namespace ICSharpCode.Decompiler.Tests
 		}
 
 		[Test, Ignore("Not implemented")]
-		public void YieldReturn([ValueSource("defaultOptions")] CompilerOptions cscOptions)
+		public void YieldReturnTest([ValueSource("defaultOptions")] CompilerOptions cscOptions)
 		{
 			Run(cscOptions: cscOptions);
 		}

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

@@ -21,7 +21,7 @@ using System.Collections.Generic;
 
 namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 {
-	public class YieldReturn
+	public class YieldReturnTest
 	{
 		int fieldOnThis;
 
@@ -213,6 +213,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 					// yield return is not allowed in catch, but yield break is
 					yield break;
 				}
+				yield return 1;
 			} finally {
 				Console.WriteLine("Finally");
 			}
@@ -228,21 +229,24 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 				} catch {
 					Console.WriteLine("Catch");
 				}
+				yield return 1;
 			} finally {
 				Console.WriteLine("Finally");
 			}
 		}
 
-		public static IEnumerable<int> YieldBreakInTryFinallyInTryFinally()
+		public static IEnumerable<int> YieldBreakInTryFinallyInTryFinally(bool b)
 		{
 			try {
 				yield return 0;
 				try {
 					Console.WriteLine("In Try");
-					yield break;
+					if (b)
+						yield break;
 				} finally {
 					Console.WriteLine("Inner Finally");
 				}
+				yield return 1;
 			} finally {
 				Console.WriteLine("Finally");
 			}