Rewrite AssignVariableNames algorithm to use variable usages instead of the list of variables.

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

@@ -250,7 +250,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 			{
 				throw null;
 			}
-			catch (Exception ex2) when (i == 0)
+			catch (Exception ex) when (i == 0)
 			{
 				Console.WriteLine("First!");
 				if (i == 1)
@@ -258,9 +258,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 					throw;
 				}
 				await Task.Yield();
-				Console.WriteLine(ex2.StackTrace);
+				Console.WriteLine(ex.StackTrace);
 			}
-			catch (Exception ex3) when (True())
+			catch (Exception ex2) when (True())
 			{
 				Console.WriteLine("Second!");
 				if (i == 1)
@@ -268,9 +268,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 					throw;
 				}
 				await Task.Yield();
-				Console.WriteLine(ex3.StackTrace);
+				Console.WriteLine(ex2.StackTrace);
 			}
-			catch (Exception ex)
+			catch (Exception ex3)
 			{
 				Console.WriteLine("Third!");
 				if (i == 1)
@@ -278,7 +278,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 					throw;
 				}
 				await Task.Yield();
-				Console.WriteLine(ex.StackTrace);
+				Console.WriteLine(ex3.StackTrace);
 			}
 			catch when (i == 0)
 			{

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

@@ -353,19 +353,19 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty.DelegateConstruction
 
 		public static void NameConflict()
 		{
-			// i is captured variable,
+			// i is local in main method,
-			// j is parameter in anonymous method
+			// j is captured variable,
+			// k is parameter in anonymous method
 			// l is local in anonymous method,
-			// k is local in main method
 			// Ensure that the decompiler doesn't introduce name conflicts
 			List<Action<int>> list = new List<Action<int>>();
-			for (int k = 0; k < 10; k++)
+			for (int i = 0; i < 10; i++)
 			{
-				int i;
+				int j;
-				for (i = 0; i < 10; i++)
+				for (j = 0; j < 10; j++)
 				{
-					list.Add(delegate (int j) {
+					list.Add(delegate (int k) {
-						for (int l = 0; l < i; l += j)
+						for (int l = 0; l < j; l += k)
 						{
 							Console.WriteLine();
 						}

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

@@ -39,10 +39,10 @@ namespace LocalFunctions
 #pragma warning disable CS0219
 				T2 t2 = default(T2);
 				object z = this;
-				for (int j = 0; j < 10; j++)
+				for (int i = 0; i < 10; i++)
 				{
-					int i = 0;
+					int i2 = 0;
-					i += NonStaticMethod6<object>(0);
+					i2 += NonStaticMethod6<object>(0);
 #if CS90
 					[My]
 					[return: My]
@@ -56,7 +56,7 @@ namespace LocalFunctions
 						return NonStaticMethod6_1<T1>() + NonStaticMethod6_1<T2>() + z.GetHashCode();
 						int NonStaticMethod6_1<T4>()
 						{
-							return i + l + NonStaticMethod6<T4>(0) + StaticMethod1<decimal>();
+							return i2 + l + NonStaticMethod6<T4>(0) + StaticMethod1<decimal>();
 						}
 					}
 				}
@@ -119,10 +119,10 @@ namespace LocalFunctions
 			{
 				T2 t2 = default(T2);
 				object z = this;
-				for (int j = 0; j < 10; j++)
+				for (int i = 0; i < 10; i++)
 				{
-					int i = 0;
+					int i2 = 0;
-					i += StaticInvokeAsFunc(NonStaticMethod6<object>);
+					i2 += StaticInvokeAsFunc(NonStaticMethod6<object>);
 					int NonStaticMethod6<T3>()
 					{
 						t2 = default(T2);
@@ -130,7 +130,7 @@ namespace LocalFunctions
 						return StaticInvokeAsFunc(NonStaticMethod6_1<T1>) + StaticInvokeAsFunc(NonStaticMethod6_1<T2>) + z.GetHashCode();
 						int NonStaticMethod6_1<T4>()
 						{
-							return i + l + StaticInvokeAsFunc(NonStaticMethod6<T4>) + StaticInvokeAsFunc(StaticMethod1<decimal>);
+							return i2 + l + StaticInvokeAsFunc(NonStaticMethod6<T4>) + StaticInvokeAsFunc(StaticMethod1<decimal>);
 						}
 					}
 				}
@@ -743,12 +743,12 @@ namespace LocalFunctions
 				int ZZZ_1()
 				{
 					t0 = 0;
-					int t = t0;
+					int t3 = t0;
 					return new Func<int>(ZZZ_1_0)();
 					int ZZZ_1_0()
 					{
 						t0 = 0;
-						t = 0;
+						t3 = 0;
 						return 0;
 					}
 				}
@@ -779,14 +779,14 @@ namespace LocalFunctions
 				int ZZZ_1()
 				{
 					t0 = 0;
-					int t1 = t0;
+					int t3 = t0;
 #if !OPT
 					Func<int> func = delegate {
 #else
 					return ((Func<int>)delegate {
 #endif
 						t0 = 0;
-						t1 = 0;
+						t3 = 0;
 						return 0;
 #if !OPT
 					};
@@ -822,14 +822,14 @@ namespace LocalFunctions
 				int ZZZ_1()
 				{
 					t0 = 0;
-					int t1 = t0;
+					int t3 = t0;
 #if !OPT
 					Func<int> func = delegate {
 #else
 					return ((Func<int>)delegate {
 #endif
 						t0 = 0;
-						t1 = 0;
+						t3 = 0;
 						return 0;
 #if !OPT
 					};

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

@@ -77,5 +77,13 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 				Console.WriteLine(*memory2);
 			}
 		}
+
+		private static void ForLoopNamingConflict(int i)
+		{
+			for (int j = 0; j < i; j++)
+			{
+				Console.WriteLine(i + " of " + j);
+			}
+		}
 	}
 }

ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.Expected.cs

@@ -96,25 +96,25 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly
 				thisField = this,
 				field1 = i
 			};
-			int field1 = default(int);
+			int field5 = default(int);
-			string field2 = default(string);
+			string field4 = default(string);
 			DisplayClass field3 = default(DisplayClass);
 			while (true)
 			{
 				switch (Rand())
 				{
 					case 1:
-						field1 = Rand();
+						field5 = Rand();
 						continue;
 					case 2:
-						field2 = Rand().ToString();
+						field4 = Rand().ToString();
 						continue;
 					case 3:
 						field3 = displayClass;
 						continue;
 				}
-				Console.WriteLine(field1);
+				Console.WriteLine(field5);
-				Console.WriteLine(field2);
+				Console.WriteLine(field4);
 				Console.WriteLine(field3);
 			}
 		}

ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs

@@ -55,7 +55,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		ILTransformContext context;
 		List<string> currentLowerCaseTypeOrMemberNames;
 		Dictionary<string, int> reservedVariableNames;
-		Dictionary<MethodDefinitionHandle, string> localFunctionMapping;
 		HashSet<ILVariable> loopCounters;
 		const char maxLoopVariableName = 'n';
 		int numDisplayClassLocals;
@@ -75,7 +74,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				currentLowerCaseTypeOrMemberNames.Add(name);
 				AddExistingName(reservedVariableNames, name);
 			}
-			localFunctionMapping = new Dictionary<MethodDefinitionHandle, string>();
 			loopCounters = CollectLoopCounters(function);
 			foreach (var f in function.Descendants.OfType<ILFunction>())
 			{
@@ -153,10 +151,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				}
 			}
 			numDisplayClassLocals = 0;
-			foreach (ILFunction f in function.Descendants.OfType<ILFunction>().Reverse())
+			PerformAssignment(function);
-			{
-				PerformAssignment(f);
-			}
 		}
 
 		static IEnumerable<string> CollectAllLowerCaseMemberNames(ITypeDefinition type)
@@ -195,11 +190,48 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 
 		void PerformAssignment(ILFunction function)
 		{
-			// remove unused variables before assigning names
+			var localFunctionMapping = new Dictionary<MethodDefinitionHandle, string>();
-			function.Variables.RemoveDead();
+			var variableMapping = new Dictionary<ILVariable, string>(ILVariableEqualityComparer.Instance);
-			Dictionary<int, string> assignedLocalSignatureIndices = new Dictionary<int, string>();
+			var assignedLocalSignatureIndices = new Dictionary<(ILFunction, int), string>();
-			foreach (var v in function.Variables.OrderBy(v => v.Name))
+
+			foreach (var inst in function.Descendants)
+			{
+				if (inst is ILFunction { Kind: ILFunctionKind.LocalFunction } localFunction)
+				{
+					// assign names to local functions
+					if (!LocalFunctionDecompiler.ParseLocalFunctionName(localFunction.Name, out _, out var newName) || !IsValidName(newName))
+						newName = null;
+					if (newName == null)
+					{
+						string nameWithoutNumber = "f";
+						if (!reservedVariableNames.TryGetValue(nameWithoutNumber, out int currentIndex))
+						{
+							currentIndex = 1;
+						}
+						int count = Math.Max(1, currentIndex) + 1;
+						reservedVariableNames[nameWithoutNumber] = count;
+						if (count > 1)
+						{
+							newName = nameWithoutNumber + count.ToString();
+						}
+						else
+						{
+							newName = nameWithoutNumber;
+						}
+					}
+					localFunction.Name = newName;
+					localFunction.ReducedMethod.Name = newName;
+					localFunctionMapping[(MethodDefinitionHandle)localFunction.ReducedMethod.MetadataToken] = newName;
+				}
+				else if (inst is IInstructionWithVariableOperand i)
+				{
+					var v = i.Variable;
+					// if there is already a valid name for the variable slot, just use it
+					if (variableMapping.TryGetValue(v, out string name))
 					{
+						v.Name = name;
+						continue;
+					}
 					switch (v.Kind)
 					{
 						case VariableKind.Parameter:
@@ -213,7 +245,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 							v.Name = "CS$<>8__locals" + (numDisplayClassLocals++);
 							break;
 						case VariableKind.Local when v.Index != null:
-						if (assignedLocalSignatureIndices.TryGetValue(v.Index.Value, out string name))
+							if (assignedLocalSignatureIndices.TryGetValue((v.Function, v.Index.Value), out name))
 							{
 								// make sure all local ILVariables that refer to the same slot in the locals signature
 								// are assigned the same name.
@@ -221,102 +253,68 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 							}
 							else
 							{
-							AssignName();
+								v.Name = AssignName(v, variableMapping);
 								// Remember the newly assigned name:
-							assignedLocalSignatureIndices.Add(v.Index.Value, v.Name);
+								assignedLocalSignatureIndices.Add((v.Function, v.Index.Value), v.Name);
 							}
 							break;
 						default:
-						AssignName();
+							v.Name = AssignName(v, variableMapping);
 							break;
 					}
-
-				void AssignName()
-				{
-					if (v.HasGeneratedName || !IsValidName(v.Name))
-					{
-						// don't use the name from the debug symbols if it looks like a generated name
-						v.Name = null;
 				}
-					else
+				else if (inst is (Call or LdFtn) and IInstructionWithMethodOperand m)
 				{
-						// use the name from the debug symbols and update index appended to duplicates
+					// update references to local functions
-						string nameWithoutNumber = SplitName(v.Name, out int newIndex);
+					if (m.Method is LocalFunctionMethod lf
-						if (!reservedVariableNames.TryGetValue(nameWithoutNumber, out int currentIndex))
+						&& localFunctionMapping.TryGetValue((MethodDefinitionHandle)lf.MetadataToken, out var name))
 					{
-							currentIndex = 1;
+						lf.Name = name;
 					}
-						reservedVariableNames[nameWithoutNumber] = Math.Max(newIndex, currentIndex);
 				}
 			}
 		}
-			foreach (var localFunction in function.LocalFunctions)
+
+		string AssignName(ILVariable v, Dictionary<ILVariable, string> variableMapping)
 		{
-				if (!LocalFunctionDecompiler.ParseLocalFunctionName(localFunction.Name, out _, out var newName) || !IsValidName(newName))
+			// variable has no valid name
-					newName = null;
+			string newName = v.Name;
-				localFunction.Name = newName;
+			if (v.HasGeneratedName || !IsValidName(newName))
-				localFunction.ReducedMethod.Name = newName;
+			{
+				// don't use the name from the debug symbols if it looks like a generated name
+				// generate a new one based on how the variable is used
+				newName = GenerateNameForVariable(v);
 			}
-			// Now generate names:
+			// use the existing name and update index appended to future conflicts
-			var mapping = new Dictionary<ILVariable, string>(ILVariableEqualityComparer.Instance);
+			string nameWithoutNumber = SplitName(newName, out int newIndex);
-			foreach (var inst in function.Descendants.OfType<IInstructionWithVariableOperand>())
+			if (reservedVariableNames.TryGetValue(nameWithoutNumber, out int lastUsedIndex))
 			{
-				var v = inst.Variable;
+				// name without number was already used
-				if (!mapping.TryGetValue(v, out string name))
+				if (v.Type.IsKnownType(KnownTypeCode.Int32) && loopCounters.Contains(v))
 				{
-					if (string.IsNullOrEmpty(v.Name))
+					// special case for loop counters,
-						v.Name = GenerateNameForVariable(v);
+					// we don't want them to be named i, i2, ..., but i, j, ...
-					mapping.Add(v, v.Name);
+					newName = GenerateNameForVariable(v);
 				}
 				else
 				{
-					v.Name = name;
+					if (newIndex > lastUsedIndex)
-				}
-			}
-			foreach (var localFunction in function.LocalFunctions)
-			{
-				var newName = localFunction.Name;
-				if (newName == null)
-				{
-					string nameWithoutNumber = "f";
-					if (!reservedVariableNames.TryGetValue(nameWithoutNumber, out int currentIndex))
-					{
-						currentIndex = 1;
-					}
-					int count = Math.Max(1, currentIndex) + 1;
-					reservedVariableNames[nameWithoutNumber] = count;
-					if (count > 1)
 					{
-						newName = nameWithoutNumber + count.ToString();
+						// new index is larger than last, so we can use it
 					}
 					else
 					{
-						newName = nameWithoutNumber;
+						// new index is smaller or equal, so we use the next value
+						newIndex = lastUsedIndex + 1;
 					}
+					// resolve conflicts by appending the index to the new name:
+					newName = nameWithoutNumber + newIndex.ToString();
 				}
-				localFunction.Name = newName;
-				localFunction.ReducedMethod.Name = newName;
-				localFunctionMapping[(MethodDefinitionHandle)localFunction.ReducedMethod.MetadataToken] = newName;
-			}
-			foreach (var inst in function.Descendants)
-			{
-				LocalFunctionMethod localFunction;
-				switch (inst)
-				{
-					case Call call:
-						localFunction = call.Method as LocalFunctionMethod;
-						break;
-					case LdFtn ldftn:
-						localFunction = ldftn.Method as LocalFunctionMethod;
-						break;
-					default:
-						localFunction = null;
-						break;
-				}
-				if (localFunction == null || !localFunctionMapping.TryGetValue((MethodDefinitionHandle)localFunction.MetadataToken, out var name))
-					continue;
-				localFunction.Name = name;
 			}
+			// update the last used index
+			reservedVariableNames[nameWithoutNumber] = newIndex;
+			variableMapping.Add(v, newName);
+			return newName;
 		}
 
 		/// <remarks>
@@ -459,23 +457,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				proposedName = GetNameByType(variable.Type);
 			}
 
-			// remove any numbers from the proposed name
+			// for generated names remove number-suffixes
-			proposedName = SplitName(proposedName, out int number);
+			return SplitName(proposedName, out _);
-
-			if (!reservedVariableNames.ContainsKey(proposedName))
-			{
-				reservedVariableNames.Add(proposedName, 0);
-			}
-			int count = ++reservedVariableNames[proposedName];
-			Debug.Assert(!string.IsNullOrWhiteSpace(proposedName));
-			if (count > 1)
-			{
-				return proposedName + count.ToString();
-			}
-			else
-			{
-				return proposedName;
-			}
 		}
 
 		static string GetNameFromInstruction(ILInstruction inst)

ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs

@@ -898,7 +898,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			}
 			context.Step("TransformCatchVariable", entryPoint.Instructions[0]);
 			exceptionVar.Kind = VariableKind.ExceptionLocal;
+			exceptionVar.Name = handler.Variable.Name;
 			exceptionVar.Type = handler.Variable.Type;
+			exceptionVar.HasGeneratedName = handler.Variable.HasGeneratedName;
 			handler.Variable = exceptionVar;
 			if (isCatchBlock)
 			{