From acdd0042a650af48ca8b4fbfa2034f67d242d390 Mon Sep 17 00:00:00 2001 From: DualFroz Date: Fri, 11 Sep 2026 23:42:13 +0200 Subject: [PATCH] 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)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 --- .../PrettyTestRunner.cs | 6 ++++ .../TestCases/Pretty/Issue3965.cs | 29 +++++++++++++++++++ .../CachedDelegateInitialization.cs | 19 +++++++++--- 3 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3965.cs diff --git a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs index faa5a74564..7145f5975d 100644 --- a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs @@ -926,6 +926,12 @@ public async Task Issue3877([ValueSource(nameof(roslyn2OrNewerWithNet40Options)) await RunForLibrary(cscOptions: cscOptions); } + [Test] + public async Task Issue3965([ValueSource(nameof(roslyn4OrNewerOptions))] CompilerOptions cscOptions) + { + await RunForLibrary(cscOptions: cscOptions); + } + [Test] public async Task AssemblyCustomAttributes([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions) { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3965.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3965.cs new file mode 100644 index 0000000000..d735cf9579 --- /dev/null +++ b/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 action) + { + } + + public static void DiscardedMethodGroupConversion() + { +#if EXPECTED_OUTPUT + new Action(M); +#else + _ = (Action)M; +#endif + } + + public static void UsedMethodGroupConversion() + { + Use(M); + } + } +} diff --git a/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs b/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs index e67f311230..48150bf5eb 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs @@ -37,7 +37,6 @@ public void Run(Block block, BlockTransformContext context) { if (CachedDelegateInitializationWithField(inst)) { - block.Instructions.RemoveAt(i); context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count; continue; } @@ -95,13 +94,25 @@ bool CachedDelegateInitializationWithField(IfInstruction inst) if (!DelegateConstruction.MatchDelegateConstruction(value.UnwrapConv(ConversionKind.Invalid) as NewObj, out _, out _, out _, true)) return false; var nextInstruction = inst.Parent.Children.ElementAtOrDefault(inst.ChildIndex + 1); - if (nextInstruction == null) - return false; - var usages = nextInstruction.Descendants.Where(i => i.MatchLdsFld(field)).ToArray(); + var usages = nextInstruction?.Descendants.Where(i => i.MatchLdsFld(field)).ToArray() ?? []; + if (usages.Length == 0) + { + // 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) return false; context.Step("CachedDelegateInitializationWithField", inst); usages[0].ReplaceWith(value); + ((Block)inst.Parent).Instructions.RemoveAt(inst.ChildIndex); context.EndStep(value); return true; }