Browse Source

Keep a discarded method-group conversion out of the delegate cache

CachedDelegateInitializationWithField only fired when the instruction
after the caching `if` read the cache field exactly once. A discarded
conversion -- `_ = (Action<int>)M;` -- reads it zero times: Roslyn emits
the null check and the store, and nothing else. The transform bailed out,
so the `<>O` cache class and its `<0>__M` field survived into the output
under names no C# compiler will accept.

Zero usages is now handled: the `if` is replaced by the delegate
construction it guarded, which is what the source expressed, and the
caching disappears with the field. The rest of the method is checked for
loads of the same field first, since only then is this `if` the sole
initialization.

Closes #3965
pull/4133/head
DualFroz 4 days ago
parent
commit
acdd0042a6
  1. 6
      ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs
  2. 29
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3965.cs
  3. 19
      ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs

6
ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs

@ -926,6 +926,12 @@ namespace ICSharpCode.Decompiler.Tests
await RunForLibrary(cscOptions: cscOptions); await RunForLibrary(cscOptions: cscOptions);
} }
[Test]
public async Task Issue3965([ValueSource(nameof(roslyn4OrNewerOptions))] CompilerOptions cscOptions)
{
await RunForLibrary(cscOptions: cscOptions);
}
[Test] [Test]
public async Task AssemblyCustomAttributes([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions) public async Task AssemblyCustomAttributes([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions)
{ {

29
ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3965.cs

@ -0,0 +1,29 @@
using System;
namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
{
internal class Issue3965
{
private static void M(int x)
{
}
private static void Use(Action<int> action)
{
}
public static void DiscardedMethodGroupConversion()
{
#if EXPECTED_OUTPUT
new Action<int>(M);
#else
_ = (Action<int>)M;
#endif
}
public static void UsedMethodGroupConversion()
{
Use(M);
}
}
}

19
ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs

@ -37,7 +37,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms
{ {
if (CachedDelegateInitializationWithField(inst)) if (CachedDelegateInitializationWithField(inst))
{ {
block.Instructions.RemoveAt(i);
context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count; context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count;
continue; continue;
} }
@ -95,13 +94,25 @@ namespace ICSharpCode.Decompiler.IL.Transforms
if (!DelegateConstruction.MatchDelegateConstruction(value.UnwrapConv(ConversionKind.Invalid) as NewObj, out _, out _, out _, true)) if (!DelegateConstruction.MatchDelegateConstruction(value.UnwrapConv(ConversionKind.Invalid) as NewObj, out _, out _, out _, true))
return false; return false;
var nextInstruction = inst.Parent.Children.ElementAtOrDefault(inst.ChildIndex + 1); var nextInstruction = inst.Parent.Children.ElementAtOrDefault(inst.ChildIndex + 1);
if (nextInstruction == null) var usages = nextInstruction?.Descendants.Where(i => i.MatchLdsFld(field)).ToArray() ?? [];
return false; if (usages.Length == 0)
var usages = nextInstruction.Descendants.Where(i => i.MatchLdsFld(field)).ToArray(); {
// A discarded method-group conversion ("_ = (Action)M;") caches the
// delegate without ever reading the cache back. Keep the conversion,
// which still allocates, and drop the caching around it -- but only
// once the rest of the method is known not to read the field either.
if (context.Function.Descendants.Any(i => i != left && i.MatchLdsFld(field)))
return false;
context.Step("CachedDelegateInitializationWithField (unused)", inst);
inst.ReplaceWith(value);
context.EndStep(value);
return true;
}
if (usages.Length != 1) if (usages.Length != 1)
return false; return false;
context.Step("CachedDelegateInitializationWithField", inst); context.Step("CachedDelegateInitializationWithField", inst);
usages[0].ReplaceWith(value); usages[0].ReplaceWith(value);
((Block)inst.Parent).Instructions.RemoveAt(inst.ChildIndex);
context.EndStep(value); context.EndStep(value);
return true; return true;
} }

Loading…
Cancel
Save