Merge pull request #4228 from LiquidSnake0/fix-4153-foreach-named-argument

Fix #4153: keep foreach when a named argument inlines the loop variable

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

@@ -16,6 +16,8 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 // DEALINGS IN THE SOFTWARE.
 
+using System.Collections.Generic;
+
 namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 {
 	public class NamedArguments
@@ -173,5 +175,35 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 			// The names are the base indexer's, which is what the call instruction names.
 			Use(((BaseNames)derived)[y: Get(1), x: Get(2)], 0, 0);
 		}
+
+		public int NamedArgsInForeachOverEnumerator(Dictionary<int, int> dict)
+		{
+			int num = 0;
+			foreach (KeyValuePair<int, int> item in dict)
+			{
+				num += Sum(y: Get(item.Key), x: Get(2));
+			}
+			return num;
+		}
+
+		public int NamedArgsInForeachOverEnumeratorInstanceCall(List<KeyValuePair<int, int>> list)
+		{
+			int num = 0;
+			foreach (KeyValuePair<int, int> item in list)
+			{
+				num += Add(y: Get(item.Value), x: Get(2));
+			}
+			return num;
+		}
+
+		public static int Sum(int x, int y)
+		{
+			return x + y;
+		}
+
+		public int Add(int x, int y)
+		{
+			return x + y;
+		}
 	}
 }

ICSharpCode.Decompiler/IL/Instructions/Block.cs

@@ -283,6 +283,9 @@ namespace ICSharpCode.Decompiler.IL
 				case BlockKind.CallInlineAssign:
 					// Allow inlining into the first instruction of the block
 					return childIndex == 0;
+				case BlockKind.CallWithNamedArgs:
+					// Allow inlining into any instruction of the block
+					return childIndex < Instructions.Count;
 				default:
 					return false;
 			}

ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@@ -942,13 +942,28 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				for (int i = 0; i < childIndex; ++i)
 				{
 					ILInstruction predecessor = inst.Parent.Children[i];
-					if (!IsSafeForInlineOver(predecessor, expressionBeingMoved))
+					if (!IsSafeForInlineOver(SkipNamedArgumentStore(predecessor), expressionBeingMoved))
 						return false;
 				}
 			}
 			return true;
 		}
 
+		/// <summary>
+		/// <see cref="StLoc"/> instructions have <see cref="InstructionFlags.MayWriteLocals"/>,
+		/// which we can ignore/skip, if the target variable is a <see cref="VariableKind.NamedArgument"/>,
+		/// because it's guaranteed that these variables are only read in the FinalInstruction.
+		/// </summary>
+		static ILInstruction SkipNamedArgumentStore(ILInstruction predecessor)
+		{
+			if (predecessor is StLoc { Variable.Kind: VariableKind.NamedArgument } store)
+			{
+				Debug.Assert(predecessor.Parent is Block { Kind: BlockKind.CallWithNamedArgs });
+				return store.Value;
+			}
+			return predecessor;
+		}
+
 		/// <summary>
 		/// Gets whether 'expressionBeingMoved' can be moved from somewhere before 'stmt' to become the replacement of 'targetLoad'.
 		/// </summary>
@@ -966,7 +981,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				for (int i = 0; i < childIndex; ++i)
 				{
 					ILInstruction predecessor = inst.Parent.Children[i];
-					if (predecessor != thisArg && !IsSafeForInlineOver(predecessor, expressionBeingMoved))
+					if (predecessor != thisArg && !IsSafeForInlineOver(SkipNamedArgumentStore(predecessor), expressionBeingMoved))
 						return false;
 				}
 			}