Fix #1881: Not properly reusing names from PDB#2

ICSharpCode.Decompiler.Tests/Helpers/Tester.cs

@@ -56,6 +56,7 @@ namespace ICSharpCode.Decompiler.Tests.Helpers
 		UseMcs = 0x20,
 		ReferenceVisualBasic = 0x40,
 		ReferenceCore = 0x80,
+		GeneratePdb = 0x100,
 	}
 
 	[Flags]
@@ -272,7 +273,7 @@ namespace ICSharpCode.Decompiler.Tests.Helpers
 					preprocessorSymbols: preprocessorSymbols.ToArray(),
 					languageVersion: Microsoft.CodeAnalysis.CSharp.LanguageVersion.CSharp8
 				);
-				var syntaxTrees = sourceFileNames.Select(f => SyntaxFactory.ParseSyntaxTree(File.ReadAllText(f), parseOptions, path: f));
+				var syntaxTrees = sourceFileNames.Select(f => SyntaxFactory.ParseSyntaxTree(File.ReadAllText(f), parseOptions, path: f, encoding: Encoding.UTF8));
 				if (flags.HasFlag(CompilerOptions.ReferenceCore)) {
 					syntaxTrees = syntaxTrees.Concat(new[] { SyntaxFactory.ParseSyntaxTree(targetFrameworkAttributeSnippet) });
 				}
@@ -296,7 +297,10 @@ namespace ICSharpCode.Decompiler.Tests.Helpers
 					));
 				CompilerResults results = new CompilerResults(new TempFileCollection());
 				results.PathToAssembly = outputFileName ?? Path.GetTempFileName();
-				var emitResult = compilation.Emit(results.PathToAssembly);
+				string pdbName = null;
+				if (flags.HasFlag(CompilerOptions.GeneratePdb))
+					pdbName = Path.ChangeExtension(outputFileName, ".pdb");
+				var emitResult = compilation.Emit(results.PathToAssembly, pdbName);
 				if (!emitResult.Success) {
 					StringBuilder b = new StringBuilder("Compiler error:");
 					foreach (var diag in emitResult.Diagnostics) {
@@ -360,7 +364,12 @@ namespace ICSharpCode.Decompiler.Tests.Helpers
 				CompilerParameters options = new CompilerParameters();
 				options.GenerateExecutable = !flags.HasFlag(CompilerOptions.Library);
 				options.CompilerOptions = "/unsafe /o" + (flags.HasFlag(CompilerOptions.Optimize) ? "+" : "-");
-				options.CompilerOptions += (flags.HasFlag(CompilerOptions.UseDebug) ? " /debug" : "");
+				string debugOption = " /debug";
+				if (flags.HasFlag(CompilerOptions.GeneratePdb)) {
+					debugOption += ":full";
+					options.IncludeDebugInformation = true;
+				}
+				options.CompilerOptions += (flags.HasFlag(CompilerOptions.UseDebug) ? debugOption : "");
 				options.CompilerOptions += (flags.HasFlag(CompilerOptions.Force32Bit) ? " /platform:anycpu32bitpreferred" : "");
 				if (preprocessorSymbols.Count > 0) {
 					options.CompilerOptions += " /d:" + string.Join(";", preprocessorSymbols);
@@ -368,7 +377,6 @@ namespace ICSharpCode.Decompiler.Tests.Helpers
 				if (outputFileName != null) {
 					options.OutputAssembly = outputFileName;
 				}
-
 				options.ReferencedAssemblies.Add("System.dll");
 				options.ReferencedAssemblies.Add("System.Core.dll");
 				options.ReferencedAssemblies.Add("System.Xml.dll");
@@ -397,7 +405,7 @@ namespace ICSharpCode.Decompiler.Tests.Helpers
 			}
 		}
 
-		public static void CompileCSharpWithPdb(string assemblyName, Dictionary<string, string> sourceFiles, PdbToXmlOptions options)
+		public static void CompileCSharpWithPdb(string assemblyName, Dictionary<string, string> sourceFiles)
 		{
 			var parseOptions = new CSharpParseOptions(languageVersion: Microsoft.CodeAnalysis.CSharp.LanguageVersion.Latest);
 
@@ -484,6 +492,9 @@ namespace ICSharpCode.Decompiler.Tests.Helpers
 				decompiler.AstTransforms.Insert(0, new RemoveCompilerAttribute());
 				decompiler.AstTransforms.Insert(0, new RemoveNamespaceMy());
 				decompiler.AstTransforms.Add(new EscapeInvalidIdentifiers());
+				var pdbFileName = Path.ChangeExtension(assemblyFileName, ".pdb");
+				if (File.Exists(pdbFileName))
+					decompiler.DebugInfoProvider = PdbProvider.DebugInfoUtils.FromFile(module, pdbFileName);
 				var syntaxTree = decompiler.DecompileWholeModuleAsSingleFile(sortTypes: true);
 
 				StringWriter output = new StringWriter();

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

@@ -49,6 +49,7 @@
     <PackageReference Include="System.Collections.Immutable" Version="1.5.0" />
     <PackageReference Include="NUnit" Version="3.12.0" />
     <PackageReference Include="System.Memory" Version="4.5.3" />
+    <PackageReference Include="Mono.Cecil" Version="0.10.3" />
   </ItemGroup>
 
   <ItemGroup>
@@ -80,6 +81,9 @@
   </ItemGroup>
 
   <ItemGroup>
+    <Compile Include="..\ICSharpCode.Decompiler.PdbProvider.Cecil\MonoCecilDebugInfoProvider.cs" Link="MonoCecilDebugInfoProvider.cs" />
+    <Compile Include="..\ILSpy\DebugInfo\DebugInfoUtils.cs" Link="DebugInfoUtils.cs" />
+    <Compile Include="..\ILSpy\DebugInfo\PortableDebugInfoProvider.cs" Link="PortableDebugInfoProvider.cs" />
     <Compile Include="DisassemblerPrettyTestRunner.cs" />
     <Compile Include="TestCases\Correctness\StringConcat.cs" />
     <Compile Include="TestCases\ILPretty\Issue1922.cs" />

ICSharpCode.Decompiler.Tests/PdbGenerationTestRunner.cs

@@ -51,7 +51,7 @@ namespace ICSharpCode.Decompiler.Tests
 			string xmlContent = File.ReadAllText(xmlFile);
 			XDocument document = XDocument.Parse(xmlContent);
 			var files = document.Descendants("file").ToDictionary(f => f.Attribute("name").Value, f => f.Value);
-			Tester.CompileCSharpWithPdb(Path.Combine(TestCasePath, testName + ".expected"), files, options);
+			Tester.CompileCSharpWithPdb(Path.Combine(TestCasePath, testName + ".expected"), files);
 
 			string peFileName = Path.Combine(TestCasePath, testName + ".expected.dll");
 			string pdbFileName = Path.Combine(TestCasePath, testName + ".expected.pdb");

ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs

@@ -284,7 +284,7 @@ namespace ICSharpCode.Decompiler.Tests
 		[Test]
 		public void VariableNaming([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions)
 		{
-			RunForLibrary(cscOptions: cscOptions);
+			RunForLibrary(cscOptions: cscOptions | CompilerOptions.GeneratePdb);
 		}
 
 		[Test]

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

@@ -1,7 +1,17 @@
-namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
+#if !OPT
+using System;
+#endif
+
+namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 {
 	internal class VariableNaming
 	{
+		private enum MyEnum
+		{
+			VALUE1 = 1,
+			VALUE2
+		}
+
 		private class C
 		{
 			public string Name;
@@ -25,5 +35,28 @@
 			string text2 = c.Text;
 #endif
 		}
+
+#if !OPT
+		private void Issue1841()
+		{
+			C gen1 = new C();
+			C gen2 = new C();
+			C gen3 = new C();
+			C gen4 = new C();
+		}
+
+		private void Issue1881()
+		{
+			MyEnum enumLocal1 = MyEnum.VALUE1;
+			MyEnum enumLocal2 = (MyEnum)0;
+			enumLocal2 = MyEnum.VALUE1;
+			object enumLocal3 = MyEnum.VALUE2;
+			object enumLocal4 = new object();
+			enumLocal4 = MyEnum.VALUE2;
+			ValueType enumLocal5 = MyEnum.VALUE1;
+			ValueType enumLocal6 = (MyEnum)0;
+			enumLocal6 = MyEnum.VALUE2;
+		}
+#endif
 	}
 }

ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs

@@ -20,12 +20,8 @@ using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Reflection;
-using System.Text;
-using System.Threading.Tasks;
 using Humanizer;
 using ICSharpCode.Decompiler.CSharp.OutputVisitor;
-using ICSharpCode.Decompiler.IL;
-using ICSharpCode.Decompiler.Semantics;
 using ICSharpCode.Decompiler.TypeSystem;
 using ICSharpCode.Decompiler.TypeSystem.Implementation;
 using ICSharpCode.Decompiler.Util;
@@ -141,6 +137,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			// remove unused variables before assigning names
 			function.Variables.RemoveDead();
 			int numDisplayClassLocals = 0;
+			Dictionary<int, string> assignedLocalSignatureIndices = new Dictionary<int, string>();
 			foreach (var v in function.Variables.OrderBy(v => v.Name)) {
 				switch (v.Kind) {
 					case VariableKind.Parameter: // ignore
@@ -151,16 +148,32 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 					case VariableKind.DisplayClassLocal:
 						v.Name = "CS$<>8__locals" + (numDisplayClassLocals++);
 						break;
-					default:
+					case VariableKind.Local when v.Index != null:
-						if (v.HasGeneratedName || !IsValidName(v.Name) || ConflictWithLocal(v)) {
+						if (assignedLocalSignatureIndices.TryGetValue(v.Index.Value, out string name)) {
-							// don't use the name from the debug symbols if it looks like a generated name
+							// make sure all local ILVariables that refer to the same slot in the locals signature
-							v.Name = null;
+							// are assigned the same name.
+							v.Name = name;
 						} else {
-							// use the name from the debug symbols
+							AssignName();
-							// (but ensure we don't use the same name for two variables)
+							// Remember the newly assigned name:
-							v.Name = GetAlternativeName(v.Name);
+							assignedLocalSignatureIndices.Add(v.Index.Value, v.Name);
 						}
 						break;
+					default:
+						AssignName();
+						break;
+				}
+
+				void AssignName()
+				{
+					if (v.HasGeneratedName || !IsValidName(v.Name) || ConflictWithLocal(v)) {
+						// don't use the name from the debug symbols if it looks like a generated name
+						v.Name = null;
+					} else {
+						// use the name from the debug symbols
+						// (but ensure we don't use the same name for two variables)
+						v.Name = GetAlternativeName(v.Name);
+					}
 				}
 			}
 			foreach (var localFunction in function.LocalFunctions) {