Re-enable foreach pattern

ICSharpCode.Decompiler/CSharp/Annotations.cs

@@ -128,12 +128,23 @@ namespace ICSharpCode.Decompiler.CSharp
 		{
 			return vi.Annotation<ILVariable>();
 		}
-		
+
+		public static ILVariable GetILVariable(this ForeachStatement loop)
+		{
+			return loop.Annotation<ILVariable>();
+		}
+
 		public static VariableInitializer WithILVariable(this VariableInitializer vi, ILVariable v)
 		{
 			vi.AddAnnotation(v);
 			return vi;
 		}
+
+		public static ForeachStatement WithILVariable(this ForeachStatement loop, ILVariable v)
+		{
+			loop.AddAnnotation(v);
+			return loop;
+		}
 	}
 	
 	public class ILVariableResolveResult : ResolveResult

ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs

@@ -266,7 +266,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 							Name = variableName,
 							Initializer = m1.Get<Expression>("initializer").Single().Detach()
 						}.CopyAnnotationsFrom(node.Expression)
-							.WithAnnotation(variable)
+						 .WithILVariable(variable)
 					}
 				}.CopyAnnotationsFrom(node);
 			} else {
@@ -313,25 +313,6 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 		// even if it results in two variables with the same name and overlapping scopes.
 		// (this issue could be fixed later by renaming one of the variables)
 		
-		// I'm not sure whether the other consumers of 'CanMoveVariableDeclarationIntoStatement' should be changed the same way.
-		bool CanMoveVariableDeclarationIntoStatement(VariableDeclarationStatement varDecl, Statement targetStatement, out Statement declarationPoint)
-		{
-			Debug.Assert(targetStatement.Ancestors.Contains(varDecl.Parent));
-			// Find all blocks between targetStatement and varDecl.Parent
-			List<BlockStatement> blocks = targetStatement.Ancestors.TakeWhile(block => block != varDecl.Parent).OfType<BlockStatement>().ToList();
-			blocks.Add((BlockStatement)varDecl.Parent); // also handle the varDecl.Parent block itself
-			blocks.Reverse(); // go from parent blocks to child blocks
-			var daa = CreateDAA(blocks[0]);
-			declarationPoint = null;
-			return false;
-			/*foreach (BlockStatement block in blocks) {
-				if (!DeclareVariables.FindDeclarationPoint(daa, varDecl, block, out declarationPoint)) {
-					return false;
-				}
-			}
-			return true;*/
-		}
-
 		private DefiniteAssignmentAnalysis CreateDAA(BlockStatement block)
 		{
 			var typeResolveContext = new CSharpTypeResolveContext(context.TypeSystem.MainAssembly);
@@ -404,23 +385,12 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 				return null;
 			}
 			VariableInitializer enumeratorVar = m.Get<VariableInitializer>("enumeratorVariable").Single();
-			IdentifierExpression itemVar = m.Get<IdentifierExpression>("itemVariable").Single();
+			var itemVar = m.Get<IdentifierExpression>("itemVariable").Single().GetILVariable();
 			WhileStatement loop = m.Get<WhileStatement>("loop").Single();
-			
-			// Find the declaration of the item variable:
-			// Because we look only outside the loop, we won't make the mistake of moving a captured variable across the loop boundary
-			VariableDeclarationStatement itemVarDecl = FindVariableDeclaration(loop, itemVar.Identifier);
-			if (itemVarDecl == null || !(itemVarDecl.Parent is BlockStatement))
-				return null;
-			
-			// Now verify that we can move the variable declaration in front of the loop:
-			Statement declarationPoint;
-			CanMoveVariableDeclarationIntoStatement(itemVarDecl, loop, out declarationPoint);
-			// We ignore the return value because we don't care whether we can move the variable into the loop
-			// (that is possible only with non-captured variables).
-			// We just care that we can move it in front of the loop:
-			if (declarationPoint != loop)
+
+			if (!VariableCanBeDeclaredInLoop(itemVar, loop)) {
 				return null;
+			}
 
 			// Make sure that the enumerator variable is not used inside the body
 			var enumeratorId = Identifier.Create(enumeratorVar.Name);
@@ -434,13 +404,14 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 				newBody.Add(stmt.Detach());
 			foreach (Statement stmt in m.Get<Statement>("statement"))
 				newBody.Add(stmt.Detach());
-			
+
+			itemVar.Kind = IL.VariableKind.ForeachLocal;
 			ForeachStatement foreachStatement = new ForeachStatement {
-				VariableType = (AstType)itemVarDecl.Type.Clone(),
-				VariableName = itemVar.Identifier,
+				VariableType = itemVar.Type.ContainsAnonymousType() ? new SimpleType("var") : context.TypeSystemAstBuilder.ConvertType(itemVar.Type),
+				VariableName = itemVar.Name,
 				InExpression = m.Get<Expression>("collection").Single().Detach(),
 				EmbeddedStatement = newBody
-			}.WithAnnotation(itemVarDecl.Variables.Single().Annotation<IL.ILVariable>());
+			}.WithILVariable(itemVar);
 			foreachStatement.CopyAnnotationsFrom(loop);
 			if (foreachStatement.InExpression is BaseReferenceExpression) {
 				foreachStatement.InExpression = new ThisReferenceExpression().CopyAnnotationsFrom(foreachStatement.InExpression);
@@ -451,6 +422,25 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 			}
 			return foreachStatement;
 		}
+
+		static bool VariableCanBeDeclaredInLoop(IL.ILVariable itemVar, WhileStatement loop)
+		{
+			if (itemVar == null || !(itemVar.Kind == IL.VariableKind.Local || itemVar.Kind == IL.VariableKind.StackSlot)) {
+				// only locals/temporaries can be converted into foreach loop variable
+				return false;
+			}
+
+			if (!itemVar.IsSingleDefinition) {
+				// foreach variable cannot be assigned to
+				return false;
+			}
+
+			if (itemVar.CaptureScope != null && itemVar.CaptureScope != loop.Annotation<IL.BlockContainer>()) {
+				// captured variables cannot be declared in the loop unless the loop is their capture scope
+				return false;
+			}
+			return true;
+		}
 		#endregion
 		
 		#region foreach (non-generic)
@@ -505,37 +495,22 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 			if (!m2.Success) return null;
 			
 			IdentifierExpression enumeratorVar = m2.Get<IdentifierExpression>("enumerator").Single();
-			IdentifierExpression itemVar = m2.Get<IdentifierExpression>("itemVar").Single();
+			var itemVar = m2.Get<IdentifierExpression>("itemVar").Single().GetILVariable();
 			WhileStatement loop = m2.Get<WhileStatement>("loop").Single();
 			
 			// verify that the getEnumeratorPattern assigns to the same variable as the nonGenericForeachPattern is reading from
 			if (!enumeratorVar.IsMatch(m1.Get("left").Single()))
 				return null;
-			
-			VariableDeclarationStatement enumeratorVarDecl = FindVariableDeclaration(loop, enumeratorVar.Identifier);
-			if (enumeratorVarDecl == null || !(enumeratorVarDecl.Parent is BlockStatement))
-				return null;
-			
-			// Find the declaration of the item variable:
-			// Because we look only outside the loop, we won't make the mistake of moving a captured variable across the loop boundary
-			VariableDeclarationStatement itemVarDecl = FindVariableDeclaration(loop, itemVar.Identifier);
-			if (itemVarDecl == null || !(itemVarDecl.Parent is BlockStatement))
-				return null;
-			
-			// Now verify that we can move the variable declaration in front of the loop:
-			Statement declarationPoint;
-			CanMoveVariableDeclarationIntoStatement(itemVarDecl, loop, out declarationPoint);
-			// We ignore the return value because we don't care whether we can move the variable into the loop
-			// (that is possible only with non-captured variables).
-			// We just care that we can move it in front of the loop:
-			if (declarationPoint != loop)
+
+			if (!VariableCanBeDeclaredInLoop(itemVar, loop))
 				return null;
 
+			itemVar.Kind = IL.VariableKind.ForeachLocal;
 			ForeachStatement foreachStatement = new ForeachStatement
 			{
-				VariableType = itemVarDecl.Type.Clone(),
-				VariableName = itemVar.Identifier,
-			}.WithAnnotation(itemVarDecl.Variables.Single().Annotation<IL.ILVariable>());
+				VariableType = itemVar.Type.ContainsAnonymousType() ? new SimpleType("var") : context.TypeSystemAstBuilder.ConvertType(itemVar.Type),
+				VariableName = itemVar.Name,
+			}.WithILVariable(itemVar);
 			BlockStatement body = new BlockStatement();
 			foreachStatement.EmbeddedStatement = body;
 			foreachStatement.CopyAnnotationsFrom(loop);
@@ -544,6 +519,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 			body.Add(node.Detach());
 			body.Add((Statement)tryCatch.Detach());
 			
+			/*
 			// Now that we moved the whole try-catch into the foreach loop; verify that we can
 			// move the enumerator into the foreach loop:
 			CanMoveVariableDeclarationIntoStatement(enumeratorVarDecl, foreachStatement, out declarationPoint);
@@ -554,7 +530,8 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 				foreachStatement.ReplaceWith(tryCatch);
 				return null;
 			}
-			
+			*/
+
 			// Now create the correct body for the foreach statement:
 			foreachStatement.InExpression = m1.Get<Expression>("collection").Single().Detach();
 			if (foreachStatement.InExpression is BaseReferenceExpression) {
@@ -566,7 +543,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 			return foreachStatement;
 		}
 		#endregion
-		
+
 		#region for
 		static readonly WhileStatement forPattern = new WhileStatement {
 			Condition = new BinaryOperatorExpression {

ICSharpCode.Decompiler/Tests/TestCases/Correctness/Capturing.cs

@@ -14,6 +14,8 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
 			TestCase2();
 			TestCase3();
 			TestCase4("TestCase4");
+			OutsideLoop();
+			InsideLoop();
 		}
 
 		static void TestCase1()
@@ -84,5 +86,40 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
 			line = v + " line";
 			return --v > 0;
 		}
+
+		static void OutsideLoop()
+		{
+			Console.WriteLine("OutsideLoop");
+			var list = new List<int> { 1, 2, 3 };
+			var functions = new List<Func<int>>();
+			using (var e = list.GetEnumerator()) {
+				int val; // declared outside loop
+						 // The decompiler cannot convert this to a foreach-loop without
+						 // changing the lambda capture semantics.
+				while (e.MoveNext()) {
+					val = e.Current;
+					functions.Add(() => val);
+				}
+			}
+			foreach (var func in functions) {
+				Console.WriteLine(func());
+			}
+		}
+
+		static void InsideLoop()
+		{
+			Console.WriteLine("InsideLoop");
+			var list = new List<int> { 1, 2, 3 };
+			var functions = new List<Func<int>>();
+			using (var e = list.GetEnumerator()) {
+				while (e.MoveNext()) {
+					int val = e.Current;
+					functions.Add(() => val);
+				}
+			}
+			foreach (var func in functions) {
+				Console.WriteLine(func());
+			}
+		}
 	}
 }

ICSharpCode.Decompiler/Tests/TestCases/Correctness/ControlFlow.cs

@@ -17,6 +17,7 @@
 // DEALINGS IN THE SOFTWARE.
 
 using System;
+using System.Collections.Generic;
 
 namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
 {
@@ -36,6 +37,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
 			Test("none", ref result);
 			Test("test", ref result);
 			Console.WriteLine(result);
+			ForeachWithAssignment(new int[] { 1, 5, 25 });
 			return 0;
 		}
 
@@ -104,5 +106,16 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
 
 			return -1;
 		}
+
+		static void ForeachWithAssignment(IEnumerable<int> inputs)
+		{
+			Console.WriteLine("ForeachWithAssignment");
+			foreach (int input in inputs) {
+				int i = input;
+				if (i < 10)
+					i *= 2;
+				Console.WriteLine(i);
+			}
+		}
 	}
 }