diff --git a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj index 697ca18d97..f1b1e5777f 100644 --- a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj @@ -93,6 +93,7 @@ + @@ -128,6 +129,7 @@ + diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/MonoFixed.cs b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/MonoFixed.cs index f3ad7c3892..c420c9d418 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/MonoFixed.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/MonoFixed.cs @@ -6,13 +6,13 @@ public unsafe void FixMultipleStrings(string text) { fixed (char* ptr = text) { - fixed (char* ptr2 = Environment.UserName) + fixed (char* userName = Environment.UserName) { - fixed (char* ptr3 = text) + fixed (char* ptr2 = text) { *ptr = 'c'; - *ptr2 = 'd'; - *ptr3 = 'e'; + *userName = 'd'; + *ptr2 = 'e'; } } } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Async.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Async.cs index 7e918263db..9ac730b3fe 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Async.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Async.cs @@ -250,7 +250,7 @@ public async Task LoadsToCatch(int i) { throw null; } - catch (Exception ex2) when (i == 0) + catch (Exception ex) when (i == 0) { Console.WriteLine("First!"); if (i == 1) @@ -258,9 +258,9 @@ public async Task LoadsToCatch(int i) 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 @@ public async Task LoadsToCatch(int i) 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 @@ public async Task LoadsToCatch(int i) throw; } await Task.Yield(); - Console.WriteLine(ex.StackTrace); + Console.WriteLine(ex3.StackTrace); } catch when (i == 0) { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateConstruction.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateConstruction.cs index 6fe5791806..c87ca9a7d0 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateConstruction.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateConstruction.cs @@ -353,19 +353,19 @@ public static Action StaticAnonymousMethodNoClosure() public static void NameConflict() { - // i is captured variable, - // j is parameter in anonymous method + // i is local in main 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> list = new List>(); - for (int k = 0; k < 10; k++) + for (int i = 0; i < 10; i++) { - int i; - for (i = 0; i < 10; i++) + int j; + for (j = 0; j < 10; j++) { - list.Add(delegate (int j) { - for (int l = 0; l < i; l += j) + list.Add(delegate (int k) { + for (int l = 0; l < j; l += k) { Console.WriteLine(); } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/LocalFunctions.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/LocalFunctions.cs index 677f6ebafe..ff29a4351c 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/LocalFunctions.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/LocalFunctions.cs @@ -39,10 +39,10 @@ public int MixedLocalFunction() where T2 : ICloneable, IConvertible #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; - i += NonStaticMethod6(0); + int i2 = 0; + i2 += NonStaticMethod6(0); #if CS90 [My] [return: My] @@ -56,7 +56,7 @@ int NonStaticMethod6(int unused) return NonStaticMethod6_1() + NonStaticMethod6_1() + z.GetHashCode(); int NonStaticMethod6_1() { - return i + l + NonStaticMethod6(0) + StaticMethod1(); + return i2 + l + NonStaticMethod6(0) + StaticMethod1(); } } } @@ -119,10 +119,10 @@ public int MixedLocalFunction2Delegate() where T2 : ICloneable, IConvertible { T2 t2 = default(T2); object z = this; - for (int j = 0; j < 10; j++) + for (int i = 0; i < 10; i++) { - int i = 0; - i += StaticInvokeAsFunc(NonStaticMethod6); + int i2 = 0; + i2 += StaticInvokeAsFunc(NonStaticMethod6); int NonStaticMethod6() { t2 = default(T2); @@ -130,7 +130,7 @@ int NonStaticMethod6() return StaticInvokeAsFunc(NonStaticMethod6_1) + StaticInvokeAsFunc(NonStaticMethod6_1) + z.GetHashCode(); int NonStaticMethod6_1() { - return i + l + StaticInvokeAsFunc(NonStaticMethod6) + StaticInvokeAsFunc(StaticMethod1); + return i2 + l + StaticInvokeAsFunc(NonStaticMethod6) + StaticInvokeAsFunc(StaticMethod1); } } } @@ -743,12 +743,12 @@ int ZZZ_0_0() int ZZZ_1() { t0 = 0; - int t = t0; + int t3 = t0; return new Func(ZZZ_1_0)(); int ZZZ_1_0() { t0 = 0; - t = 0; + t3 = 0; return 0; } } @@ -779,14 +779,14 @@ int ZZZ_0() int ZZZ_1() { t0 = 0; - int t1 = t0; + int t3 = t0; #if !OPT Func func = delegate { #else return ((Func)delegate { #endif t0 = 0; - t1 = 0; + t3 = 0; return 0; #if !OPT }; @@ -822,14 +822,14 @@ int ZZZ_0() int ZZZ_1() { t0 = 0; - int t1 = t0; + int t3 = t0; #if !OPT Func func = delegate { #else return ((Func)delegate { #endif t0 = 0; - t1 = 0; + t3 = 0; return 0; #if !OPT }; diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs index 9901fd69e1..3b993096e7 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs @@ -431,9 +431,9 @@ public unsafe void UseFixedMemberAsReference(StructWithFixedSizeMembers* m) public unsafe void PinFixedMember(ref StructWithFixedSizeMembers m) { - fixed (int* ptr = m.Integers) + fixed (int* integers = m.Integers) { - UsePointer(ptr); + UsePointer(integers); } } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/VariableNamingWithoutSymbols.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/VariableNamingWithoutSymbols.cs index e724559fd1..1de5982ba9 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/VariableNamingWithoutSymbols.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/VariableNamingWithoutSymbols.cs @@ -1,4 +1,7 @@ -namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty +using System; +using System.Collections.Generic; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty { internal class VariableNamingWithoutSymbols { @@ -25,5 +28,62 @@ private void Test2(string text, C c) string text2 = c.Text; #endif } + + private static IDisposable GetData() + { + return null; + } + + private static void UseData(IDisposable data) + { + + } + + private static IEnumerable GetItems() + { + throw null; + } + + private static byte[] GetMemory() + { + throw null; + } + + private static void Test(int item) + { + foreach (int item2 in GetItems()) + { + Console.WriteLine(item2); + } + } + + private static void Test(IDisposable data) + { +#if CS80 + using IDisposable data2 = GetData(); + UseData(data2); +#else + using (IDisposable data2 = GetData()) + { + UseData(data2); + } +#endif + } + + private unsafe static void Test(byte[] memory) + { + fixed (byte* memory2 = GetMemory()) + { + Console.WriteLine(*memory2); + } + } + + private static void ForLoopNamingConflict(int i) + { + for (int j = 0; j < i; j++) + { + Console.WriteLine(i + " of " + j); + } + } } } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.Expected.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.Expected.cs index cfa266a46d..3eb91b54ab 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.Expected.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.Expected.cs @@ -96,25 +96,25 @@ public void Issue1898(int i) thisField = this, field1 = i }; - int field1 = default(int); - string field2 = default(string); + int field5 = default(int); + 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(field2); + Console.WriteLine(field5); + Console.WriteLine(field4); Console.WriteLine(field3); } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs b/ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs index d84091d9d2..9b832955cb 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs @@ -55,7 +55,6 @@ public class AssignVariableNames : IILTransform ILTransformContext context; List currentLowerCaseTypeOrMemberNames; Dictionary reservedVariableNames; - Dictionary localFunctionMapping; HashSet loopCounters; const char maxLoopVariableName = 'n'; int numDisplayClassLocals; @@ -75,7 +74,6 @@ public void Run(ILFunction function, ILTransformContext context) currentLowerCaseTypeOrMemberNames.Add(name); AddExistingName(reservedVariableNames, name); } - localFunctionMapping = new Dictionary(); loopCounters = CollectLoopCounters(function); foreach (var f in function.Descendants.OfType()) { @@ -153,10 +151,7 @@ public void Run(ILFunction function, ILTransformContext context) } } numDisplayClassLocals = 0; - foreach (ILFunction f in function.Descendants.OfType().Reverse()) - { - PerformAssignment(f); - } + PerformAssignment(function); } static IEnumerable CollectAllLowerCaseMemberNames(ITypeDefinition type) @@ -195,128 +190,131 @@ bool IsSetOrEventAccessor(IMethod method) void PerformAssignment(ILFunction function) { - // remove unused variables before assigning names - function.Variables.RemoveDead(); - Dictionary assignedLocalSignatureIndices = new Dictionary(); - foreach (var v in function.Variables.OrderBy(v => v.Name)) + var localFunctionMapping = new Dictionary(); + var variableMapping = new Dictionary(ILVariableEqualityComparer.Instance); + var assignedLocalSignatureIndices = new Dictionary<(ILFunction, int), string>(); + + foreach (var inst in function.Descendants) { - switch (v.Kind) + if (inst is ILFunction { Kind: ILFunctionKind.LocalFunction } localFunction) { - case VariableKind.Parameter: - // Parameter names are handled in ILReader.CreateILVariable - // and CSharpDecompiler.FixParameterNames - break; - case VariableKind.InitializerTarget: // keep generated names - AddExistingName(reservedVariableNames, v.Name); - break; - case VariableKind.DisplayClassLocal: - v.Name = "CS$<>8__locals" + (numDisplayClassLocals++); - break; - case VariableKind.Local when v.Index != null: - if (assignedLocalSignatureIndices.TryGetValue(v.Index.Value, out string name)) + // 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)) { - // make sure all local ILVariables that refer to the same slot in the locals signature - // are assigned the same name. - v.Name = name; + currentIndex = 1; + } + int count = Math.Max(1, currentIndex) + 1; + reservedVariableNames[nameWithoutNumber] = count; + if (count > 1) + { + newName = nameWithoutNumber + count.ToString(); } else { - AssignName(); - // Remember the newly assigned name: - assignedLocalSignatureIndices.Add(v.Index.Value, v.Name); + newName = nameWithoutNumber; } - break; - default: - AssignName(); - break; + } + localFunction.Name = newName; + localFunction.ReducedMethod.Name = newName; + localFunctionMapping[(MethodDefinitionHandle)localFunction.ReducedMethod.MetadataToken] = newName; } - - void AssignName() + else if (inst is IInstructionWithVariableOperand i) { - if (v.HasGeneratedName || !IsValidName(v.Name) || ConflictWithLocal(v)) + 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)) { - // don't use the name from the debug symbols if it looks like a generated name - v.Name = null; + v.Name = name; + continue; } - else + switch (v.Kind) { - // use the name from the debug symbols and update index appended to duplicates - string nameWithoutNumber = SplitName(v.Name, out int newIndex); - if (!reservedVariableNames.TryGetValue(nameWithoutNumber, out int currentIndex)) - { - currentIndex = 1; - } - reservedVariableNames[nameWithoutNumber] = Math.Max(newIndex, currentIndex); + case VariableKind.Parameter: + // Parameter names are handled in ILReader.CreateILVariable + // and CSharpDecompiler.FixParameterNames + break; + case VariableKind.InitializerTarget: // keep generated names + AddExistingName(reservedVariableNames, v.Name); + break; + case VariableKind.DisplayClassLocal: + v.Name = "CS$<>8__locals" + (numDisplayClassLocals++); + break; + case VariableKind.Local when v.Index != null: + 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. + v.Name = name; + } + else + { + v.Name = AssignName(v, variableMapping); + // Remember the newly assigned name: + assignedLocalSignatureIndices.Add((v.Function, v.Index.Value), v.Name); + } + break; + default: + v.Name = AssignName(v, variableMapping); + break; + } + } + else if (inst is (Call or LdFtn) and IInstructionWithMethodOperand m) + { + // update references to local functions + if (m.Method is LocalFunctionMethod lf + && localFunctionMapping.TryGetValue((MethodDefinitionHandle)lf.MetadataToken, out var name)) + { + lf.Name = name; } } } - foreach (var localFunction in function.LocalFunctions) + } + + string AssignName(ILVariable v, Dictionary variableMapping) + { + // variable has no valid name + string newName = v.Name; + if (v.HasGeneratedName || !IsValidName(newName)) { - if (!LocalFunctionDecompiler.ParseLocalFunctionName(localFunction.Name, out _, out var newName) || !IsValidName(newName)) - newName = null; - localFunction.Name = 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: - var mapping = new Dictionary(ILVariableEqualityComparer.Instance); - foreach (var inst in function.Descendants.OfType()) + // use the existing name and update index appended to future conflicts + string nameWithoutNumber = SplitName(newName, out int newIndex); + if (reservedVariableNames.TryGetValue(nameWithoutNumber, out int lastUsedIndex)) { - var v = inst.Variable; - if (!mapping.TryGetValue(v, out string name)) + // name without number was already used + if (v.Type.IsKnownType(KnownTypeCode.Int32) && loopCounters.Contains(v)) { - if (string.IsNullOrEmpty(v.Name)) - v.Name = GenerateNameForVariable(v); - mapping.Add(v, v.Name); + // special case for loop counters, + // we don't want them to be named i, i2, ..., but i, j, ... + newName = GenerateNameForVariable(v); } else { - v.Name = name; - } - } - 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) + if (newIndex > lastUsedIndex) { - 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; } /// @@ -326,6 +324,7 @@ internal static bool IsSupportedInstruction(object arg) { switch (arg) { + case GetPinnableReference _: case LdObj _: case LdFlda _: case LdsFlda _: @@ -336,16 +335,6 @@ internal static bool IsSupportedInstruction(object arg) } } - bool ConflictWithLocal(ILVariable v) - { - if (v.Kind == VariableKind.UsingLocal || v.Kind == VariableKind.ForeachLocal) - { - if (reservedVariableNames.ContainsKey(v.Name)) - return true; - } - return false; - } - internal static bool IsValidName(string varName) { if (string.IsNullOrWhiteSpace(varName)) @@ -431,6 +420,12 @@ string GenerateNameForVariable(ILVariable variable) if (!currentLowerCaseTypeOrMemberNames.Contains(name)) proposedNameForStores.Add(name); } + else if (store is PinnedRegion pinnedRegion) + { + var name = GetNameFromInstruction(pinnedRegion.Init); + if (!currentLowerCaseTypeOrMemberNames.Contains(name)) + proposedNameForStores.Add(name); + } } if (proposedNameForStores.Count == 1) { @@ -462,32 +457,21 @@ string GenerateNameForVariable(ILVariable variable) proposedName = GetNameByType(variable.Type); } - // remove any numbers from the proposed name - proposedName = SplitName(proposedName, out int number); - - 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; - } + // for generated names remove number-suffixes + return SplitName(proposedName, out _); } static string GetNameFromInstruction(ILInstruction inst) { switch (inst) { + case GetPinnableReference getPinnableReference: + return GetNameFromInstruction(getPinnableReference.Argument); case LdObj ldobj: return GetNameFromInstruction(ldobj.Target); case LdFlda ldflda: + if (ldflda.Field.IsCompilerGeneratedOrIsInCompilerGeneratedClass()) + return GetNameFromInstruction(ldflda.Target); return CleanUpVariableName(ldflda.Field.Name); case LdsFlda ldsflda: return CleanUpVariableName(ldsflda.Field.Name); @@ -567,6 +551,8 @@ static bool ExcludeMethodFromCandidates(IMethod m) return true; if (m.Name == "Concat" && m.DeclaringType.IsKnownType(KnownTypeCode.String)) return true; + if (m.Name == "GetPinnableReference") + return true; return false; } diff --git a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs index c3b464399e..ed086d68f2 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs @@ -898,7 +898,9 @@ void TransformCatchVariable(TryCatchHandler handler, Block entryPoint, bool isCa } 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) {