Browse Source

Fix #1572: parameters of lambdas and local functions are renamed, if there are with names from outer scopes collisions.

pull/3416/head
Siegfried Pammer 3 months ago
parent
commit
ffcd468d22
  1. 50
      ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs
  2. 4
      ICSharpCode.Decompiler.Tests/TestCases/ILPretty/GuessAccessors.cs
  3. 33
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateConstruction.cs
  4. 12
      ICSharpCode.Decompiler.Tests/TestCases/VBPretty/Async.cs
  5. 2
      ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs
  6. 2
      ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs
  7. 42
      ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs

50
ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs

@ -233,22 +233,21 @@ namespace ICSharpCode.Decompiler.Tests @@ -233,22 +233,21 @@ namespace ICSharpCode.Decompiler.Tests
[Test]
public async Task ExceptionHandling([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions)
{
await RunForLibrary(cscOptions: cscOptions, decompilerSettings: new DecompilerSettings {
NullPropagation = false,
await RunForLibrary(cscOptions: cscOptions, configureDecompiler: settings => {
settings.NullPropagation = false;
// legacy csc generates a dead store in debug builds
RemoveDeadStores = (cscOptions == CompilerOptions.None),
FileScopedNamespaces = false,
settings.RemoveDeadStores = (cscOptions == CompilerOptions.None);
settings.FileScopedNamespaces = false;
});
}
[Test]
public async Task Switch([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions)
{
await RunForLibrary(cscOptions: cscOptions, decompilerSettings: new DecompilerSettings {
await RunForLibrary(cscOptions: cscOptions, configureDecompiler: settings => {
// legacy csc generates a dead store in debug builds
RemoveDeadStores = (cscOptions == CompilerOptions.None),
SwitchExpressions = false,
FileScopedNamespaces = false,
settings.RemoveDeadStores = (cscOptions == CompilerOptions.None);
settings.SwitchExpressions = false;
});
}
@ -267,7 +266,10 @@ namespace ICSharpCode.Decompiler.Tests @@ -267,7 +266,10 @@ namespace ICSharpCode.Decompiler.Tests
[Test]
public async Task DelegateConstruction([ValueSource(nameof(defaultOptionsWithMcs))] CompilerOptions cscOptions)
{
await RunForLibrary(cscOptions: cscOptions);
await RunForLibrary(cscOptions: cscOptions, configureDecompiler: settings => {
settings.QueryExpressions = false;
settings.NullableReferenceTypes = false;
});
}
[Test]
@ -293,9 +295,9 @@ namespace ICSharpCode.Decompiler.Tests @@ -293,9 +295,9 @@ namespace ICSharpCode.Decompiler.Tests
{
await RunForLibrary(
cscOptions: cscOptions,
decompilerSettings: new DecompilerSettings {
UseEnhancedUsing = false,
FileScopedNamespaces = false,
configureDecompiler: settings => {
settings.UseEnhancedUsing = false;
settings.FileScopedNamespaces = false;
}
);
}
@ -327,11 +329,11 @@ namespace ICSharpCode.Decompiler.Tests @@ -327,11 +329,11 @@ namespace ICSharpCode.Decompiler.Tests
[Test]
public async Task Loops([ValueSource(nameof(defaultOptionsWithMcs))] CompilerOptions cscOptions)
{
DecompilerSettings settings = Tester.GetSettings(cscOptions);
await RunForLibrary(cscOptions: cscOptions, configureDecompiler: settings => {
// legacy csc generates a dead store in debug builds
settings.RemoveDeadStores = (cscOptions == CompilerOptions.None);
settings.UseExpressionBodyForCalculatedGetterOnlyProperties = false;
await RunForLibrary(cscOptions: cscOptions, decompilerSettings: settings);
});
}
[Test]
@ -440,9 +442,7 @@ namespace ICSharpCode.Decompiler.Tests @@ -440,9 +442,7 @@ namespace ICSharpCode.Decompiler.Tests
[Test]
public async Task VariableNamingWithoutSymbols([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions)
{
var settings = Tester.GetSettings(cscOptions);
settings.UseDebugSymbols = false;
await RunForLibrary(cscOptions: cscOptions, decompilerSettings: settings);
await RunForLibrary(cscOptions: cscOptions, configureDecompiler: settings => settings.UseDebugSymbols = false);
}
[Test]
@ -474,7 +474,7 @@ namespace ICSharpCode.Decompiler.Tests @@ -474,7 +474,7 @@ namespace ICSharpCode.Decompiler.Tests
{
await RunForLibrary(
cscOptions: cscOptions,
decompilerSettings: new DecompilerSettings { UseEnhancedUsing = false, FileScopedNamespaces = false }
configureDecompiler: settings => { settings.UseEnhancedUsing = false; }
);
}
@ -499,7 +499,7 @@ namespace ICSharpCode.Decompiler.Tests @@ -499,7 +499,7 @@ namespace ICSharpCode.Decompiler.Tests
[Test]
public async Task FileScopedNamespaces([ValueSource(nameof(roslyn4OrNewerOptions))] CompilerOptions cscOptions)
{
await RunForLibrary(cscOptions: cscOptions, decompilerSettings: new DecompilerSettings());
await RunForLibrary(cscOptions: cscOptions, configureDecompiler: settings => settings.FileScopedNamespaces = true);
}
[Test]
@ -601,7 +601,7 @@ namespace ICSharpCode.Decompiler.Tests @@ -601,7 +601,7 @@ namespace ICSharpCode.Decompiler.Tests
[Test]
public async Task Issue1080([ValueSource(nameof(roslynOnlyOptions))] CompilerOptions cscOptions)
{
await RunForLibrary(cscOptions: cscOptions, decompilerSettings: new DecompilerSettings(CSharp.LanguageVersion.CSharp6));
await RunForLibrary(cscOptions: cscOptions, configureDecompiler: settings => settings.SetLanguageVersion(CSharp.LanguageVersion.CSharp6));
}
[Test]
@ -712,12 +712,12 @@ namespace ICSharpCode.Decompiler.Tests @@ -712,12 +712,12 @@ namespace ICSharpCode.Decompiler.Tests
await RunForLibrary(cscOptions: cscOptions);
}
async Task RunForLibrary([CallerMemberName] string testName = null, AssemblerOptions asmOptions = AssemblerOptions.None, CompilerOptions cscOptions = CompilerOptions.None, DecompilerSettings decompilerSettings = null)
async Task RunForLibrary([CallerMemberName] string testName = null, AssemblerOptions asmOptions = AssemblerOptions.None, CompilerOptions cscOptions = CompilerOptions.None, Action<DecompilerSettings> configureDecompiler = null)
{
await Run(testName, asmOptions | AssemblerOptions.Library, cscOptions | CompilerOptions.Library, decompilerSettings);
await Run(testName, asmOptions | AssemblerOptions.Library, cscOptions | CompilerOptions.Library, configureDecompiler);
}
async Task Run([CallerMemberName] string testName = null, AssemblerOptions asmOptions = AssemblerOptions.None, CompilerOptions cscOptions = CompilerOptions.None, DecompilerSettings decompilerSettings = null)
async Task Run([CallerMemberName] string testName = null, AssemblerOptions asmOptions = AssemblerOptions.None, CompilerOptions cscOptions = CompilerOptions.None, Action<DecompilerSettings> configureDecompiler = null)
{
var csFile = Path.Combine(TestCasePath, testName + ".cs");
var exeFile = TestsAssemblyOutput.GetFilePath(TestCasePath, testName, Tester.GetSuffix(cscOptions) + ".exe");
@ -739,7 +739,9 @@ namespace ICSharpCode.Decompiler.Tests @@ -739,7 +739,9 @@ namespace ICSharpCode.Decompiler.Tests
}
// 2. Decompile
var decompiled = await Tester.DecompileCSharp(exeFile, decompilerSettings ?? Tester.GetSettings(cscOptions)).ConfigureAwait(false);
var settings = Tester.GetSettings(cscOptions);
configureDecompiler?.Invoke(settings);
var decompiled = await Tester.DecompileCSharp(exeFile, settings).ConfigureAwait(false);
// 3. Compile
CodeAssert.FilesAreEqual(csFile, decompiled, Tester.GetPreprocessorSymbols(cscOptions).Append("EXPECTED_OUTPUT").ToArray());

4
ICSharpCode.Decompiler.Tests/TestCases/ILPretty/GuessAccessors.cs

@ -15,8 +15,8 @@ namespace ClassLibrary1 @@ -15,8 +15,8 @@ namespace ClassLibrary1
//IL_0007: Expected O, but got Unknown
UnknownClass val = new UnknownClass();
int? unknownProperty = val.UnknownProperty;
int? num2 = (val.UnknownProperty = unknownProperty.GetValueOrDefault());
int? num3 = num2;
int? num = (val.UnknownProperty = unknownProperty.GetValueOrDefault());
int? num3 = num;
List<object> list = new List<object> {
val[unknownProperty.Value] ?? "",
val.NotProperty,

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

@ -19,15 +19,33 @@ @@ -19,15 +19,33 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;
#if CS100
using System.Threading.Tasks;
#endif
namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty.DelegateConstruction
{
public static class DelegateConstruction
{
internal class Dummy
{
public int baz;
public List<Dummy> more;
}
[CompilerGenerated]
internal class Helper
{
internal bool HelpMe(Dummy dum)
{
return true;
}
}
private class InstanceTests
{
public struct SomeData
@ -643,6 +661,21 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty.DelegateConstruction @@ -643,6 +661,21 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty.DelegateConstruction
{
del(x);
}
public void Issue1572(DelegateConstruction.Dummy dum)
{
#if EXPECTED_OUTPUT
DelegateConstruction.Helper CS_0024_003C_003E8__locals0 = new DelegateConstruction.Helper();
DelegateConstruction.Dummy dummy = dum.more.Where((DelegateConstruction.Dummy dummy2) => true).Where((DelegateConstruction.Dummy dummy2) => true).FirstOrDefault();
Console.WriteLine();
dummy.baz++;
#else
DelegateConstruction.Helper h = new DelegateConstruction.Helper();
DelegateConstruction.Dummy localDummy = dum.more.Where(h.HelpMe).Where(h.HelpMe).FirstOrDefault();
Console.WriteLine();
localDummy.baz++;
#endif
}
}
[AttributeUsage(AttributeTargets.All)]

12
ICSharpCode.Decompiler.Tests/TestCases/VBPretty/Async.cs

@ -40,9 +40,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.VBPretty @@ -40,9 +40,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.VBPretty
public async void AwaitDefaultYieldAwaitable()
{
#if LEGACY_VBC || (OPTIMIZE && !ROSLYN4)
YieldAwaitable yieldAwaitable = default(YieldAwaitable);
YieldAwaitable yieldAwaitable2 = yieldAwaitable;
await yieldAwaitable2;
YieldAwaitable yieldAwaitable2 = default(YieldAwaitable);
YieldAwaitable yieldAwaitable = yieldAwaitable2;
await yieldAwaitable;
#else
await default(YieldAwaitable);
#endif
@ -51,9 +51,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.VBPretty @@ -51,9 +51,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.VBPretty
public async void AwaitDefaultHopToThreadPool()
{
#if LEGACY_VBC || (OPTIMIZE && !ROSLYN4)
HopToThreadPoolAwaitable hopToThreadPoolAwaitable = default(HopToThreadPoolAwaitable);
HopToThreadPoolAwaitable hopToThreadPoolAwaitable2 = hopToThreadPoolAwaitable;
await hopToThreadPoolAwaitable2;
HopToThreadPoolAwaitable hopToThreadPoolAwaitable2 = default(HopToThreadPoolAwaitable);
HopToThreadPoolAwaitable hopToThreadPoolAwaitable = hopToThreadPoolAwaitable2;
await hopToThreadPoolAwaitable;
#else
await default(HopToThreadPoolAwaitable);
#endif

2
ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@ -1282,7 +1282,7 @@ namespace ICSharpCode.Decompiler.CSharp @@ -1282,7 +1282,7 @@ namespace ICSharpCode.Decompiler.CSharp
{
if (string.IsNullOrWhiteSpace(parameter.Name) && !parameter.Type.IsArgList())
{
// needs to be consistent with logic in ILReader.CreateILVarable
// needs to be consistent with logic in ILReader.CreateILVariable
parameter.Name = "P_" + i;
}
i++;

2
ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@ -2554,7 +2554,7 @@ namespace ICSharpCode.Decompiler.CSharp @@ -2554,7 +2554,7 @@ namespace ICSharpCode.Decompiler.CSharp
}
if (string.IsNullOrEmpty(pd.Name) && !pd.Type.IsArgList())
{
// needs to be consistent with logic in ILReader.CreateILVarable(ParameterDefinition)
// needs to be consistent with logic in ILReader.CreateILVariable
pd.Name = "P_" + i;
}
if (settings.AnonymousTypes && parameter.Type.ContainsAnonymousType())

42
ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs

@ -179,7 +179,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -179,7 +179,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
string name = p.Name;
if (string.IsNullOrWhiteSpace(name) && p.Type != SpecialType.ArgList)
{
// needs to be consistent with logic in ILReader.CreateILVarable
// needs to be consistent with logic in ILReader.CreateILVariable
name = "P_" + i;
}
if (variables.TryGetValue(i, out var v))
@ -221,6 +221,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -221,6 +221,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms
if (variables.TryGetValue(i, out var v))
variableMapping[v] = p.Name;
}
else if (variables.TryGetValue(i, out var v))
{
v.HasGeneratedName = true;
}
}
}
}
@ -287,6 +291,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -287,6 +291,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return loopCounters.Contains(v) || (parentScope?.IsLoopCounter(v) == true);
}
public string AssignNameIfUnassigned(ILVariable v)
{
if (variableMapping.TryGetValue(v, out var name))
return name;
return AssignName(v);
}
public string AssignName(ILVariable v)
{
// variable has no valid name
@ -432,22 +443,26 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -432,22 +443,26 @@ namespace ICSharpCode.Decompiler.IL.Transforms
function.AcceptVisitor(this, null);
}
protected override Unit Default(ILInstruction inst, VariableScope context)
Unit VisitChildren(ILInstruction inst, VariableScope context)
{
foreach (var child in inst.Children)
{
child.AcceptVisitor(this, context);
}
if (inst is not IInstructionWithVariableOperand { Variable: var v })
return default;
}
protected override Unit Default(ILInstruction inst, VariableScope context)
{
if (inst is IInstructionWithVariableOperand { Variable: var v })
{
// if there is already a valid name for the variable slot, just use it
string name = context.TryGetExistingName(v);
if (!string.IsNullOrEmpty(name))
{
v.Name = name;
return default;
return VisitChildren(inst, context);
}
switch (v.Kind)
@ -460,6 +475,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -460,6 +475,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
case VariableKind.NamedArgument:
context.ReserveVariableName(v.Name);
break;
case VariableKind.UsingLocal when v.AddressCount == 0 && v.LoadCount == 0:
// using variables that are not read, will not be declared in source source
break;
case VariableKind.DisplayClassLocal:
v.Name = context.NextDisplayClassLocal();
break;
@ -481,8 +499,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -481,8 +499,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
v.Name = context.AssignName(v);
break;
}
}
return default;
return VisitChildren(inst, context);
}
protected internal override Unit VisitILFunction(ILFunction function, VariableScope context)
@ -515,7 +534,18 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -515,7 +534,18 @@ namespace ICSharpCode.Decompiler.IL.Transforms
context.Add((MethodDefinitionHandle)function.ReducedMethod.MetadataToken, newName);
}
return base.VisitILFunction(function, new VariableScope(function, this.context, context));
var nestedContext = new VariableScope(function, this.context, context);
base.VisitILFunction(function, nestedContext);
if (function.Kind != ILFunctionKind.TopLevelFunction)
{
foreach (var p in function.Variables.Where(v => v.Kind == VariableKind.Parameter && v.Index >= 0))
{
p.Name = nestedContext.AssignNameIfUnassigned(p);
}
}
return default;
}
protected internal override Unit VisitCall(Call inst, VariableScope context)

Loading…
Cancel
Save