Declare variables in the correct location.

ICSharpCode.Decompiler/Ast/AstMethodBodyBuilder.cs

@@ -18,7 +18,7 @@ namespace Decompiler
 		MethodDefinition methodDef;
 		TypeSystem typeSystem;
 		DecompilerContext context;
-		HashSet<ILVariable> definedLocalVars = new HashSet<ILVariable>();
+		HashSet<ILVariable> localVariablesToDefine = new HashSet<ILVariable>(); // local variables that are missing a definition
 		
 		public static BlockStatement CreateMethodBody(MethodDefinition methodDef, DecompilerContext context)
 		{
@@ -65,6 +65,10 @@ namespace Decompiler
 			context.CancellationToken.ThrowIfCancellationRequested();
 			Ast.BlockStatement astBlock = TransformBlock(ilMethod);
 			CommentStatement.ReplaceAll(astBlock); // convert CommentStatements to Comments
+			foreach (ILVariable v in localVariablesToDefine) {
+				DeclareVariableInSmallestScope.DeclareVariable(astBlock, AstBuilder.ConvertType(v.Type), v.Name);
+			}
+			
 			return astBlock;
 		}
 		
@@ -607,19 +611,14 @@ namespace Decompiler
 					}
 					case Code.Rethrow: return new Ast.ThrowStatement();
 					case Code.Sizeof: return new Ast.SizeOfExpression { Type = AstBuilder.ConvertType(operand as TypeReference) };
-					case Code.Starg: 
-						return new Ast.AssignmentExpression(new Ast.IdentifierExpression(((ParameterDefinition)operand).Name).WithAnnotation(operand), arg1);
+				case Code.Starg:
+					return new Ast.AssignmentExpression(new Ast.IdentifierExpression(((ParameterDefinition)operand).Name).WithAnnotation(operand), arg1);
 					case Code.Stloc: {
 						ILVariable locVar = (ILVariable)operand;
-						if (!definedLocalVars.Contains(locVar)) {
-							definedLocalVars.Add(locVar);
-							return new Ast.VariableDeclarationStatement(
-								locVar.Type != null ? AstBuilder.ConvertType(locVar.Type) : new Ast.PrimitiveType("var"),
-								locVar.Name,
-								arg1);
-						} else {
-							return new Ast.AssignmentExpression(new Ast.IdentifierExpression(locVar.Name).WithAnnotation(locVar), arg1);
+						if (!localVariablesToDefine.Contains(locVar)) {
+							localVariablesToDefine.Add(locVar);
 						}
+						return new Ast.AssignmentExpression(new Ast.IdentifierExpression(locVar.Name).WithAnnotation(locVar), arg1);
 					}
 					case Code.Stobj: return InlineAssembly(byteCode, args);
 					case Code.Switch: return InlineAssembly(byteCode, args);

ICSharpCode.Decompiler/Ast/DeclareVariableInSmallestScope.cs

@@ -0,0 +1,73 @@
+// Copyright (c) AlphaSierraPapa for the SharpDevelop Team (for details please see \doc\copyright.txt)
+// This code is distributed under MIT X11 license (for details please see \doc\license.txt)
+
+using System;
+using System.Linq;
+using ICSharpCode.NRefactory.CSharp;
+using ICSharpCode.NRefactory.CSharp.PatternMatching;
+
+namespace Decompiler
+{
+	/// <summary>
+	/// Helper class for declaring variables.
+	/// </summary>
+	public static class DeclareVariableInSmallestScope
+	{
+		static readonly ExpressionStatement assignmentPattern = new ExpressionStatement(
+			new AssignmentExpression(
+				new NamedNode("ident", new IdentifierExpression()).ToExpression(),
+				new AnyNode("init").ToExpression()
+			));
+		
+		/// <summary>
+		/// Declares a variable in the smallest required scope.
+		/// </summary>
+		/// <param name="node">The root of the subtree being searched for the best insertion position</param>
+		/// <param name="type">The type of the new variable</param>
+		/// <param name="name">The name of the new variable</param>
+		/// <param name="allowPassIntoLoops">Whether the variable is allowed to be placed inside a loop</param>
+		public static void DeclareVariable(AstNode node, AstType type, string name, bool allowPassIntoLoops = true)
+		{
+			AstNode pos = FindInsertPos(node, name, allowPassIntoLoops);
+			if (pos != null) {
+				Match m = assignmentPattern.Match(pos);
+				if (m != null && m.Get<IdentifierExpression>("ident").Single().Identifier == name) {
+					pos.ReplaceWith(new VariableDeclarationStatement(type, name, m.Get<Expression>("init").Single().Detach()));
+				} else {
+					pos.Parent.InsertChildBefore(pos, new VariableDeclarationStatement(type, name), BlockStatement.StatementRole);
+				}
+			}
+		}
+		
+		static AstNode FindInsertPos(AstNode node, string name, bool allowPassIntoLoops)
+		{
+			IdentifierExpression ident = node as IdentifierExpression;
+			if (ident != null && ident.Identifier == name && ident.TypeArguments.Count == 0)
+				return node;
+			
+			AstNode pos = null;
+			AstNode withinPos = null;
+			while (node != null) {
+				AstNode withinCurrent = FindInsertPos(node.FirstChild, name, allowPassIntoLoops);
+				if (withinCurrent != null) {
+					if (pos == null) {
+						pos = node;
+						withinPos = withinCurrent;
+					} else {
+						return pos;
+					}
+				}
+				node = node.NextSibling;
+			}
+			if (withinPos != null && withinPos.Role == BlockStatement.StatementRole && (allowPassIntoLoops || !IsLoop(pos)))
+				return withinPos;
+			else
+				return pos;
+		}
+		
+		static bool IsLoop(AstNode node)
+		{
+			return node is ForStatement || node is ForeachStatement || node is DoWhileStatement || node is WhileStatement;
+		}
+	}
+}

ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs

@@ -175,63 +175,75 @@ namespace Decompiler.Transforms
 				}
 				if (!ok)
 					continue;
-				Dictionary<string, Expression> dict = new Dictionary<string, Expression>();
+				Dictionary<FieldReference, AstNode> dict = new Dictionary<FieldReference, AstNode>();
 				// Delete the variable declaration statement:
-				AstNode cur;
-				AstNode next = stmt.NextSibling;
+				AstNode cur = stmt.NextSibling;
 				stmt.Remove();
-				for (cur = next; cur != null; cur = next) {
-					next = cur.NextSibling;
-					
-					// Delete any following statements as long as they assign parameters to the display class:
-					// Test for the pattern:
-					// "variableName.MemberName = right;"
-					ExpressionStatement closureFieldAssignmentPattern = new ExpressionStatement(
-						new AssignmentExpression(
-							new NamedNode("left", new MemberReferenceExpression { Target = new IdentifierExpression(variable.Name) }).ToExpression(),
-							new AnyNode("right").ToExpression()
-						)
-					);
-					Match m = closureFieldAssignmentPattern.Match(cur);
-					if (m != null && IsParameter(m.Get<Expression>("right").Single())) {
-						dict[m.Get<MemberReferenceExpression>("left").Single().MemberName] = m.Get<Expression>("right").Single();
-						cur.Remove();
-					} else {
-						break;
+				if (blockStatement.Parent.NodeType == NodeType.Member || blockStatement.Parent is Accessor) {
+					// Delete any following statements as long as they assign parameters to the display class
+					// Do parameter handling only for closures created in the top scope (direct child of method/accessor)
+					List<ParameterReference> parameterOccurrances = blockStatement.Descendants.OfType<IdentifierExpression>()
+						.Select(n => n.Annotation<ParameterReference>()).Where(p => p != null).ToList();
+					AstNode next;
+					for (; cur != null; cur = next) {
+						next = cur.NextSibling;
+						
+						// Test for the pattern:
+						// "variableName.MemberName = right;"
+						ExpressionStatement closureFieldAssignmentPattern = new ExpressionStatement(
+							new AssignmentExpression(
+								new NamedNode("left", new MemberReferenceExpression { Target = new IdentifierExpression(variable.Name) }).ToExpression(),
+								new AnyNode("right").ToExpression()
+							)
+						);
+						Match m = closureFieldAssignmentPattern.Match(cur);
+						if (m != null) {
+							AstNode right = m.Get("right").Single();
+							bool isParameter = false;
+							if (right is ThisReferenceExpression) {
+								isParameter = true;
+							} else if (right is IdentifierExpression) {
+								// handle parameters only if the whole method contains no other occurrance except for 'right'
+								ParameterReference param = right.Annotation<ParameterReference>();
+								isParameter = parameterOccurrances.Count(c => c == param) == 1;
+							}
+							if (isParameter) {
+								dict[m.Get<MemberReferenceExpression>("left").Single().Annotation<FieldReference>()] = right;
+								cur.Remove();
+							} else {
+								break;
+							}
+						} else {
+							break;
+						}
 					}
 				}
 				
-				// Now create variables for all fields of the display class (except for those that we already handled)
+				// Now create variables for all fields of the display class (except for those that we already handled as parameters)
+				List<Tuple<AstType, string>> variablesToDeclare = new List<Tuple<AstType, string>>();
 				foreach (FieldDefinition field in type.Fields) {
-					if (dict.ContainsKey(field.Name))
+					if (dict.ContainsKey(field))
 						continue;
-					VariableDeclarationStatement newVarDecl = new VariableDeclarationStatement();
-					newVarDecl.Type = AstBuilder.ConvertType(field.FieldType, field);
-					newVarDecl.Variables.Add(new VariableInitializer(field.Name));
-					blockStatement.InsertChildBefore(cur, newVarDecl, BlockStatement.StatementRole);
-					dict[field.Name] = new IdentifierExpression(field.Name);
+					variablesToDeclare.Add(Tuple.Create(AstBuilder.ConvertType(field.FieldType, field), field.Name));
+					dict[field] = new IdentifierExpression(field.Name);
 				}
 				
 				// Now figure out where the closure was accessed and use the simpler replacement expression there:
 				foreach (var identExpr in blockStatement.Descendants.OfType<IdentifierExpression>()) {
 					if (identExpr.Identifier == variable.Name) {
 						MemberReferenceExpression mre = (MemberReferenceExpression)identExpr.Parent;
-						Expression replacement;
-						if (dict.TryGetValue(mre.MemberName, out replacement)) {
+						AstNode replacement;
+						if (dict.TryGetValue(mre.Annotation<FieldReference>(), out replacement)) {
 							mre.ReplaceWith(replacement.Clone());
 						}
 					}
 				}
+				// Now insert the variable declarations (we can do this after the replacements only so that the scope detection works):
+				foreach (var tuple in variablesToDeclare) {
+					DeclareVariableInSmallestScope.DeclareVariable(blockStatement, tuple.Item1, tuple.Item2, allowPassIntoLoops: false);
+				}
 			}
 			return null;
 		}
-		
-		bool IsParameter(Expression expr)
-		{
-			if (expr is ThisReferenceExpression)
-				return true;
-			IdentifierExpression ident = expr as IdentifierExpression;
-			return ident != null && ident.Annotation<ParameterReference>() != null;
-		}
 	}
 }

ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj

@@ -52,6 +52,7 @@
     <Compile Include="Ast\AstBuilder.cs" />
     <Compile Include="Ast\AstMethodBodyBuilder.cs" />
     <Compile Include="Ast\CommentStatement.cs" />
+    <Compile Include="Ast\DeclareVariableInSmallestScope.cs" />
     <Compile Include="Ast\DecompilerContext.cs" />
     <Compile Include="Ast\NameVariables.cs" />
     <Compile Include="Ast\NRefactoryExtensions.cs" />