From 833bb5809c1d2482ec1a031d7d3ae2d5ffbf6887 Mon Sep 17 00:00:00 2001 From: Marcus Mikelic <29722978+samoriental@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:43:44 -0400 Subject: [PATCH 1/3] [Pretty Tests] add two tests for delegate caching --- .../PrettyTestRunner.cs | 6 +++ .../TestCases/Pretty/DelegateCaching.cs | 40 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCaching.cs diff --git a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs index faa5a7456..6ddbc2f5f 100644 --- a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs @@ -332,6 +332,12 @@ namespace ICSharpCode.Decompiler.Tests }); } + [Test] + public async Task DelegateCaching([ValueSource(nameof(roslyn4OrNewerOptions))] CompilerOptions cscOptions) + { + await RunForLibrary(cscOptions: cscOptions); + } + [Test] public async Task AnonymousTypes([ValueSource(nameof(defaultOptionsWithMcs))] CompilerOptions cscOptions) { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCaching.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCaching.cs new file mode 100644 index 000000000..8484a4930 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCaching.cs @@ -0,0 +1,40 @@ +// Copyright (c) 2026 marcusmalloc +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty +{ + internal static class DelegateCaching + { + // Issue #3921: explicit construction must preserve a fresh delegate on each call. + public static Action FreshDelegate() + { + return new Action(M); + } + + public static Action CachedDelegate() + { + return M; + } + + private static void M() + { + } + } +} From 0ab608f9814015c166c23067165cd0e91507f0bb Mon Sep 17 00:00:00 2001 From: Marcus Mikelic <29722978+samoriental@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:30:05 -0400 Subject: [PATCH 2/3] [Decompiler] gpt-slop copy --- .../CSharp/ExpressionBuilder.cs | 5 + .../CSharp/TranslatedExpression.cs | 11 ++ .../FlowAnalysis/DataFlowVisitor.cs | 9 ++ ICSharpCode.Decompiler/IL/Instructions.cs | 123 ++++++++++++++++++ ICSharpCode.Decompiler/IL/Instructions.tt | 3 + .../CachedDelegateInitialization.cs | 27 ++-- global.json | 2 +- 7 files changed, 166 insertions(+), 14 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 58546162b..67867375f 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -558,6 +558,11 @@ namespace ICSharpCode.Decompiler.CSharp return new CallBuilder(this, typeSystem, settings).Build(inst, context.TypeHint); } + protected internal override TranslatedExpression VisitCachedDelegate(CachedDelegate inst, TranslationContext context) + { + return Translate(inst.Argument, context.TypeHint).WithILInstruction(inst); + } + protected internal override TranslatedExpression VisitLdVirtDelegate(LdVirtDelegate inst, TranslationContext context) { return new CallBuilder(this, typeSystem, settings).Build(inst); diff --git a/ICSharpCode.Decompiler/CSharp/TranslatedExpression.cs b/ICSharpCode.Decompiler/CSharp/TranslatedExpression.cs index d682da2eb..58c5018b2 100644 --- a/ICSharpCode.Decompiler/CSharp/TranslatedExpression.cs +++ b/ICSharpCode.Decompiler/CSharp/TranslatedExpression.cs @@ -27,6 +27,7 @@ using ICSharpCode.Decompiler.CSharp.Transforms; using ICSharpCode.Decompiler.IL; using ICSharpCode.Decompiler.Semantics; using ICSharpCode.Decompiler.TypeSystem; +using ICSharpCode.Decompiler.TypeSystem.Implementation; using ICSharpCode.Decompiler.Util; #nullable enable @@ -270,6 +271,16 @@ namespace ICSharpCode.Decompiler.CSharp else if (Expression is ObjectCreateExpression oce && conversion.Conversion.IsMethodGroupConversion && oce.Arguments.Count == 1 && expressionBuilder.settings.UseImplicitMethodGroupConversion) { + // C# 11 caches static method groups. Keep explicit construction when the IL creates a fresh delegate. + if (conversion.Conversion.Method.IsStatic + && conversion.Conversion.Method is not LocalFunctionMethod { IsStaticLocalFunction: false } + && conversion.Conversion.Method.Parameters.Count == type.GetDelegateInvokeMethod()?.Parameters.Count + && expressionBuilder.settings.GetMinimumRequiredVersion() >= LanguageVersion.CSharp11_0 + && expressionBuilder.currentFunction.Kind != ILFunctionKind.ExpressionTree + && !ILInstructions.Any(i => i is CachedDelegate)) + { + return this; + } return this.UnwrapChild(oce.Arguments.Single()); } break; diff --git a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs index 82d9560b1..e0bcba5c5 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs @@ -754,6 +754,15 @@ namespace ICSharpCode.Decompiler.FlowAnalysis HandleBinaryWithOptionalEvaluation(inst, inst.ValueInst, inst.FallbackInst); } + protected internal override void VisitCachedDelegate(CachedDelegate inst) + { + DebugStartPoint(inst); + State cachedState = state.Clone(); + inst.Argument.AcceptVisitor(this); + state.JoinWith(cachedState); + DebugEndPoint(inst); + } + protected internal override void VisitDynamicLogicOperatorInstruction(DynamicLogicOperatorInstruction inst) { HandleBinaryWithOptionalEvaluation(inst, inst.Left, inst.Right); diff --git a/ICSharpCode.Decompiler/IL/Instructions.cs b/ICSharpCode.Decompiler/IL/Instructions.cs index 18072653c..604c58c95 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.cs +++ b/ICSharpCode.Decompiler/IL/Instructions.cs @@ -177,6 +177,8 @@ namespace ICSharpCode.Decompiler.IL UnboxAny, /// Creates an object instance and calls the constructor. NewObj, + /// Reuses a cached delegate, evaluating the argument only when the cache is empty. + CachedDelegate, /// Creates an array instance. NewArr, /// Returns the default value for a type. @@ -4580,6 +4582,103 @@ namespace ICSharpCode.Decompiler.IL } } namespace ICSharpCode.Decompiler.IL +{ + /// Reuses a cached delegate, evaluating the argument only when the cache is empty. + public sealed partial class CachedDelegate : ILInstruction + { + public CachedDelegate(ILInstruction argument) : base(OpCode.CachedDelegate) + { + this.Argument = argument; + } + public static readonly SlotInfo ArgumentSlot = new SlotInfo("Argument"); + ILInstruction argument = null!; + public ILInstruction Argument { + get { return this.argument; } + set { + ValidateChild(value); + SetChildInstruction(ref this.argument, value, 0); + } + } + protected sealed override int GetChildCount() + { + return 1; + } + protected sealed override ILInstruction GetChild(int index) + { + switch (index) + { + case 0: + return this.argument; + default: + throw new IndexOutOfRangeException(); + } + } + protected sealed override void SetChild(int index, ILInstruction value) + { + switch (index) + { + case 0: + this.Argument = value; + break; + default: + throw new IndexOutOfRangeException(); + } + } + protected sealed override SlotInfo GetChildSlot(int index) + { + switch (index) + { + case 0: + return ArgumentSlot; + default: + throw new IndexOutOfRangeException(); + } + } + public sealed override ILInstruction Clone() + { + var clone = (CachedDelegate)ShallowClone(); + clone.Argument = this.argument.Clone(); + return clone; + } + public override StackType ResultType => Argument.ResultType; + public override IType InferType(ICompilation compilation) => Argument.InferType(compilation); + protected override InstructionFlags ComputeFlags() + { + return argument.Flags | InstructionFlags.ControlFlow; + } + public override InstructionFlags DirectFlags { + get { + return InstructionFlags.ControlFlow; + } + } + protected override void WriteToCore(ITextOutput output, ILAstWritingOptions options) + { + WriteILRange(output, options); + output.Write(OpCode); + output.Write('('); + this.argument.WriteTo(output, options); + output.Write(')'); + } + public override void AcceptVisitor(ILVisitor visitor) + { + visitor.VisitCachedDelegate(this); + } + public override T AcceptVisitor(ILVisitor visitor) + { + return visitor.VisitCachedDelegate(this); + } + public override T AcceptVisitor(ILVisitor visitor, C context) + { + return visitor.VisitCachedDelegate(this, context); + } + protected internal override bool PerformMatch(ILInstruction? other, ref Patterns.Match match) + { + var o = other as CachedDelegate; + return o != null && this.argument.PerformMatch(o.argument, ref match); + } + } +} +namespace ICSharpCode.Decompiler.IL { /// Creates an array instance. public sealed partial class NewArr : ILInstruction @@ -7412,6 +7511,10 @@ namespace ICSharpCode.Decompiler.IL { Default(inst); } + protected internal virtual void VisitCachedDelegate(CachedDelegate inst) + { + Default(inst); + } protected internal virtual void VisitNewArr(NewArr inst) { Default(inst); @@ -7822,6 +7925,10 @@ namespace ICSharpCode.Decompiler.IL { return Default(inst); } + protected internal virtual T VisitCachedDelegate(CachedDelegate inst) + { + return Default(inst); + } protected internal virtual T VisitNewArr(NewArr inst) { return Default(inst); @@ -8232,6 +8339,10 @@ namespace ICSharpCode.Decompiler.IL { return Default(inst, context); } + protected internal virtual T VisitCachedDelegate(CachedDelegate inst, C context) + { + return Default(inst, context); + } protected internal virtual T VisitNewArr(NewArr inst, C context) { return Default(inst, context); @@ -8433,6 +8544,7 @@ namespace ICSharpCode.Decompiler.IL "unbox", "unbox.any", "newobj", + "cached.delegate", "newarr", "default.value", "throw", @@ -9003,6 +9115,17 @@ namespace ICSharpCode.Decompiler.IL type = default(IType); return false; } + public bool MatchCachedDelegate([NotNullWhen(true)] out ILInstruction? argument) + { + var inst = this as CachedDelegate; + if (inst != null) + { + argument = inst.Argument; + return true; + } + argument = default(ILInstruction); + return false; + } public bool MatchNewArr([NotNullWhen(true)] out IType? type) { var inst = this as NewArr; diff --git a/ICSharpCode.Decompiler/IL/Instructions.tt b/ICSharpCode.Decompiler/IL/Instructions.tt index 8a20bff22..bfac4f2da 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.tt +++ b/ICSharpCode.Decompiler/IL/Instructions.tt @@ -287,6 +287,9 @@ Unary, HasTypeOperand, MemoryAccess, MayThrow, ResultType("this.type")), new OpCode("newobj", "Creates an object instance and calls the constructor.", CustomClassName("NewObj"), Call, ResultType("Method.DeclaringType")), + new OpCode("cached.delegate", "Reuses a cached delegate, evaluating the argument only when the cache is empty.", + CustomChildren(new [] { new ChildInfo("argument") }), ControlFlow, + ResultType("Argument.InferType(compilation)", "Argument.ResultType")), new OpCode("newarr", "Creates an array instance.", CustomClassName("NewArr"), HasTypeOperand, CustomChildren(new [] { new ArgumentInfo("indices") { IsCollection = true } }, true), diff --git a/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs b/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs index e67f31123..93941cdbe 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs @@ -79,7 +79,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// } /// ... one usage of CachedAnonMethodDelegate ... /// => - /// ... one usage of DelegateConstruction ... + /// ... one usage of cached.delegate(DelegateConstruction) ... /// bool CachedDelegateInitializationWithField(IfInstruction inst) { @@ -101,7 +101,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (usages.Length != 1) return false; context.Step("CachedDelegateInitializationWithField", inst); - usages[0].ReplaceWith(value); + usages[0].ReplaceWith(new CachedDelegate(value)); context.EndStep(value); return true; } @@ -111,7 +111,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// stloc v(DelegateConstruction) /// } /// => - /// stloc v(DelegateConstruction) + /// stloc v(cached.delegate(DelegateConstruction)) /// bool CachedDelegateInitializationWithLocal(IfInstruction inst) { @@ -141,6 +141,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; context.Step("CachedDelegateInitializationWithLocal", inst); ((Block)otherStore.Parent).Instructions.Remove(otherStore); + ((StLoc)storeInst).Value = new CachedDelegate(value); inst.ReplaceWith(storeInst); context.EndStep(storeInst); return true; @@ -152,7 +153,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// stloc s(stobj(ldsflda(CachedAnonMethodDelegate), DelegateConstruction)) /// } /// => - /// stloc s(DelegateConstruction) + /// stloc s(cached.delegate(DelegateConstruction)) /// bool CachedDelegateInitializationRoslynInStaticWithLocal(IfInstruction inst) { @@ -174,7 +175,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!DelegateConstruction.MatchDelegateConstruction((NewObj)stobj.Value, out _, out _, out _, true)) return false; context.Step("CachedDelegateInitializationRoslynInStaticWithLocal", inst); - storeBeforeIf.Value = stobj.Value; + storeBeforeIf.Value = new CachedDelegate(stobj.Value); return true; } @@ -184,7 +185,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// stloc s(stobj(ldflda(CachedAnonMethodDelegate), DelegateConstruction)) /// } /// => - /// stloc s(DelegateConstruction) + /// stloc s(cached.delegate(DelegateConstruction)) /// bool CachedDelegateInitializationRoslynWithLocal(IfInstruction inst) { @@ -206,7 +207,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!DelegateConstruction.MatchDelegateConstruction((NewObj)stobj.Value, out _, out _, out _, true)) return false; context.Step("CachedDelegateInitializationRoslynWithLocal", inst); - storeBeforeIf.Value = stobj.Value; + storeBeforeIf.Value = new CachedDelegate(stobj.Value); return true; } @@ -217,7 +218,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// stloc s(ldobj System.Action(ldsflda $I4-1)) /// } /// => - /// stloc s(DelegateConstruction) + /// stloc s(cached.delegate(DelegateConstruction)) /// bool CachedDelegateInitializationVB(IfInstruction inst) { @@ -249,7 +250,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!DelegateConstruction.MatchDelegateConstruction(delegateConstruction, out _, out _, out _, true)) return false; context.Step("CachedDelegateInitializationVB", inst); - var stloc = new StLoc(s, delegateConstruction); + var stloc = new StLoc(s, new CachedDelegate(delegateConstruction)); inst.ReplaceWith(stloc); context.EndStep(stloc); return true; @@ -261,7 +262,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// } /// leave IL_0005 (stsfld CachedAnonMethodDelegate(DelegateConstruction)) /// => - /// leave IL_0005 (DelegateConstruction) + /// leave IL_0005 (cached.delegate(DelegateConstruction)) /// bool CachedDelegateInitializationVBWithReturn(IfInstruction inst) { @@ -279,7 +280,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!DelegateConstruction.MatchDelegateConstruction(delegateConstruction, out _, out _, out _, true)) return false; context.Step("CachedDelegateInitializationVBWithReturn", inst); - leaveAfterIf.Value = delegateConstruction; + leaveAfterIf.Value = new CachedDelegate(delegateConstruction); return true; } @@ -290,7 +291,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// stloc s(stobj delegateType(ldflda CachedAnonMethodDelegate(ldloc closure), DelegateConstruction)) /// } /// => - /// stloc s(DelegateConstruction) + /// stloc s(cached.delegate(DelegateConstruction)) /// bool CachedDelegateInitializationVBWithClosure(IfInstruction inst) { @@ -322,7 +323,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!DelegateConstruction.MatchDelegateConstruction(delegateConstruction, out _, out _, out _, true)) return false; context.Step("CachedDelegateInitializationVBWithClosure", inst); - var stloc = new StLoc(s, delegateConstruction); + var stloc = new StLoc(s, new CachedDelegate(delegateConstruction)); inst.ReplaceWith(stloc); context.EndStep(stloc); return true; diff --git a/global.json b/global.json index 115ca7301..58a3053ca 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "11.0.0", + "version": "11.0.100-preview.7.26381.103", "rollForward": "major", "allowPrerelease": true }, From 701df21df67501d82caedcad5614cd3bf5d9a906 Mon Sep 17 00:00:00 2001 From: Marcus Mikelic <29722978+samoriental@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:20:13 -0400 Subject: [PATCH 3/3] Preserve delegate during decomp --- .../PrettyTestRunner.cs | 23 +++- .../Pretty/AnonymousTypeMethodGroups.cs | 48 ++++++++ .../TestCases/Pretty/DelegateCaching.cs | 12 ++ .../DelegateCachingWithExplicitConversions.cs | 39 +++++++ ICSharpCode.Decompiler/CSharp/CallBuilder.cs | 45 +++---- .../CSharp/ExpressionBuilder.cs | 28 ++++- .../CSharp/TranslatedExpression.cs | 22 ++-- .../FlowAnalysis/DataFlowVisitor.cs | 18 ++- .../CachedDelegateInitialization.cs | 110 +++++++----------- global.json | 2 +- 10 files changed, 219 insertions(+), 128 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Pretty/AnonymousTypeMethodGroups.cs create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCachingWithExplicitConversions.cs diff --git a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs index 6ddbc2f5f..013fab62f 100644 --- a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs @@ -338,12 +338,25 @@ namespace ICSharpCode.Decompiler.Tests await RunForLibrary(cscOptions: cscOptions); } + [Test] + public async Task DelegateCachingWithExplicitConversions([ValueSource(nameof(roslyn4OrNewerOptions))] CompilerOptions cscOptions) + { + await RunForLibrary(cscOptions: cscOptions, configureDecompiler: settings => settings.UseImplicitMethodGroupConversion = false); + } + [Test] public async Task AnonymousTypes([ValueSource(nameof(defaultOptionsWithMcs))] CompilerOptions cscOptions) { await RunForLibrary(cscOptions: cscOptions); } + [Test] + public async Task AnonymousTypeMethodGroups([ValueSource(nameof(roslyn3OrNewerOptions))] CompilerOptions cscOptions) + { + // Older compilers emit uncached method groups even when decompiling to a language version that supports caching. + await RunForLibrary(cscOptions: cscOptions, settings: new DecompilerSettings { FileScopedNamespaces = false }); + } + [Test] public async Task StringConcatenation([ValueSource(nameof(roslyn3OrNewerOptions))] CompilerOptions cscOptions) { @@ -679,6 +692,8 @@ namespace ICSharpCode.Decompiler.Tests public async Task FunctionPointers([ValueSource(nameof(roslyn3OrNewerOptions))] CompilerOptions cscOptions) { await RunForLibrary(cscOptions: cscOptions); + // Disabling implicit delegate conversions must not introduce function-pointer casts. + await RunForLibrary(cscOptions: cscOptions, configureDecompiler: settings => settings.UseImplicitMethodGroupConversion = false); } [Test] @@ -1070,12 +1085,12 @@ namespace ICSharpCode.Decompiler.Tests await RunForLibrary(cscOptions: cscOptions); } - async Task RunForLibrary([CallerMemberName] string testName = null, AssemblerOptions asmOptions = AssemblerOptions.None, CompilerOptions cscOptions = CompilerOptions.None, Action configureDecompiler = null) + async Task RunForLibrary([CallerMemberName] string testName = null, AssemblerOptions asmOptions = AssemblerOptions.None, CompilerOptions cscOptions = CompilerOptions.None, Action configureDecompiler = null, DecompilerSettings settings = null) { - await Run(testName, asmOptions | AssemblerOptions.Library, cscOptions | CompilerOptions.Library, configureDecompiler); + await Run(testName, asmOptions | AssemblerOptions.Library, cscOptions | CompilerOptions.Library, configureDecompiler, settings); } - async Task Run([CallerMemberName] string testName = null, AssemblerOptions asmOptions = AssemblerOptions.None, CompilerOptions cscOptions = CompilerOptions.None, Action configureDecompiler = null) + async Task Run([CallerMemberName] string testName = null, AssemblerOptions asmOptions = AssemblerOptions.None, CompilerOptions cscOptions = CompilerOptions.None, Action configureDecompiler = null, DecompilerSettings settings = null) { var csFile = Path.Combine(TestCasePath, testName + ".cs"); var exeFile = TestsAssemblyOutput.GetFilePath(TestCasePath, testName, Tester.GetSuffix(cscOptions) + ".exe"); @@ -1097,7 +1112,7 @@ namespace ICSharpCode.Decompiler.Tests } // 2. Decompile - var settings = Tester.GetSettings(cscOptions); + settings ??= Tester.GetSettings(cscOptions); configureDecompiler?.Invoke(settings); var decompiled = await Tester.DecompileCSharp(exeFile, settings).ConfigureAwait(false); diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/AnonymousTypeMethodGroups.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/AnonymousTypeMethodGroups.cs new file mode 100644 index 000000000..71e942def --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/AnonymousTypeMethodGroups.cs @@ -0,0 +1,48 @@ +// Copyright (c) 2026 marcusmalloc +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System.Linq; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty +{ + internal static class AnonymousTypeMethodGroups + { + public static object AnonymousTypeArgument() + { + return new[] { + new { + X = 1 + } + }.Select(Identity).Single(); + } + + public static object AnonymousArrayTypeArgument() + { + return new[] { new[] { + new { + X = 1 + } + } }.Select(Identity).Single(); + } + + private static T Identity(T value) + { + return value; + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCaching.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCaching.cs index 8484a4930..83a9b9544 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCaching.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCaching.cs @@ -22,6 +22,8 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty { internal static class DelegateCaching { + public delegate void CustomDelegate(); + // Issue #3921: explicit construction must preserve a fresh delegate on each call. public static Action FreshDelegate() { @@ -33,6 +35,16 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty return M; } + public static object FreshDelegateAsObject() + { + return new CustomDelegate(M); + } + + public static object CachedDelegateAsObject() + { + return (CustomDelegate)M; + } + private static void M() { } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCachingWithExplicitConversions.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCachingWithExplicitConversions.cs new file mode 100644 index 000000000..a7a9974b9 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DelegateCachingWithExplicitConversions.cs @@ -0,0 +1,39 @@ +// Copyright (c) 2026 marcusmalloc +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty +{ + internal static class DelegateCachingWithExplicitConversions + { + public static Action FreshDelegate() + { + return new Action(M); + } + + public static Action CachedDelegate() + { + return (Action)M; + } + + private static void M() + { + } + } +} diff --git a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs index d4cb8e090..2ae3465b9 100644 --- a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs @@ -2037,7 +2037,12 @@ namespace ICSharpCode.Decompiler.CSharp } } - private bool CanUseDelegateConstruction(IMethod targetMethod, ILInstruction thisArg, IMethod invokeMethod) + static bool IsBoundExtensionMethod(IMethod method, IMethod? invokeMethod) + { + return method.IsExtensionMethod && method.Parameters.Count - 1 == invokeMethod?.Parameters.Count; + } + + private static bool CanUseDelegateConstruction(IMethod targetMethod, ILInstruction thisArg, IMethod? invokeMethod) { // Accessors cannot be directly referenced as method group in C# // see https://github.com/icsharpcode/ILSpy/issues/1741#issuecomment-540179101 @@ -2045,36 +2050,17 @@ namespace ICSharpCode.Decompiler.CSharp return false; if (targetMethod.IsStatic) { - // If the invoke method is known, we can compare the parameter counts to figure out whether the - // delegate is static or binds the first argument - if (invokeMethod != null) - { - if (invokeMethod.Parameters.Count == targetMethod.Parameters.Count) - { - return thisArg.MatchLdNull(); - } - else if (targetMethod.IsExtensionMethod && invokeMethod.Parameters.Count == targetMethod.Parameters.Count - 1) - { - return true; - } - else - { - return false; - } - } - else + if (invokeMethod == null) { - // delegate type unknown: + // Delegate type unknown. return thisArg.MatchLdNull() || targetMethod.IsExtensionMethod; } + // An unbound static delegate supplies every method parameter through Invoke. + if (invokeMethod.Parameters.Count == targetMethod.Parameters.Count) + return thisArg.MatchLdNull(); + return IsBoundExtensionMethod(targetMethod, invokeMethod); } - else - { - // targetMethod is instance method - if (invokeMethod != null && invokeMethod.Parameters.Count != targetMethod.Parameters.Count) - return false; - return true; - } + return invokeMethod == null || invokeMethod.Parameters.Count == targetMethod.Parameters.Count; } internal TranslatedExpression Build(LdVirtDelegate inst) @@ -2124,7 +2110,7 @@ namespace ICSharpCode.Decompiler.CSharp Debug.Assert(localFunction != null); return (default, addTypeArguments: true, localFunction.Name!, ToMethodGroup(method, localFunction)); } - if (method.IsExtensionMethod && method.Parameters.Count - 1 == invokeMethod?.Parameters.Count) + if (IsBoundExtensionMethod(method, invokeMethod)) { IType targetType = method.Parameters[0].Type; if (targetType.Kind == TypeKind.ByReference && thisArg is Box thisArgBox) @@ -2242,13 +2228,14 @@ namespace ICSharpCode.Decompiler.CSharp TranslatedExpression HandleDelegateConstruction(IType delegateType, IMethod method, ExpectedTargetDetails expectedTargetDetails, ILInstruction thisArg, ILInstruction inst) { var invokeMethod = delegateType.GetDelegateInvokeMethod(); + bool capturesFirstArgument = !method.IsStatic || IsBoundExtensionMethod(method, invokeMethod); var targetExpression = BuildDelegateReference(method, invokeMethod, expectedTargetDetails, thisArg); var oce = new ObjectCreateExpression(expressionBuilder.ConvertType(delegateType), targetExpression) .WithILInstruction(inst) .WithRR(new ConversionResolveResult( delegateType, targetExpression.ResolveResult, - Conversion.MethodGroupConversion(method, expectedTargetDetails.CallOpCode == OpCode.CallVirt, false))); + Conversion.MethodGroupConversion(method, expectedTargetDetails.CallOpCode == OpCode.CallVirt, capturesFirstArgument))); return oce; } diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 67867375f..74a66600f 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -560,7 +560,33 @@ namespace ICSharpCode.Decompiler.CSharp protected internal override TranslatedExpression VisitCachedDelegate(CachedDelegate inst, TranslationContext context) { - return Translate(inst.Argument, context.TypeHint).WithILInstruction(inst); + var expression = Translate(inst.Argument, context.TypeHint); + if (expression.Expression is ObjectCreateExpression objectCreation && objectCreation.Arguments.Count == 1 + && expression.ResolveResult is ConversionResolveResult { Conversion.IsMethodGroupConversion: true }) + { + // A method-group conversion allows caching; explicit construction would allocate. + // ConvertTo can remove the cast when the context supplies the delegate type. + var cast = new CastExpression(objectCreation.Type.Detach(), objectCreation.Arguments.Single().Detach()) + .CopyAnnotationsFrom(objectCreation); + return new TranslatedExpression(cast, expression.ResolveResult).WithILInstruction(inst); + } + return expression.WithILInstruction(inst); + } + + /// + /// Gets whether the C# compiler would cache a method-group conversion in the current context. + /// + internal bool MethodGroupConversionWouldBeCached(Conversion conversion) + { + if (settings.GetMinimumRequiredVersion() < LanguageVersion.CSharp11_0 + || currentFunction.Kind == ILFunctionKind.ExpressionTree + || decompilationContext.CurrentMember is { SymbolKind: SymbolKind.Constructor, IsStatic: true }) + { + return false; + } + // Local-function symbols report IsStatic even when the C# declaration cannot be static. + return !conversion.DelegateCapturesFirstArgument + && conversion.Method is not LocalFunctionMethod { IsStaticLocalFunction: false }; } protected internal override TranslatedExpression VisitLdVirtDelegate(LdVirtDelegate inst, TranslationContext context) diff --git a/ICSharpCode.Decompiler/CSharp/TranslatedExpression.cs b/ICSharpCode.Decompiler/CSharp/TranslatedExpression.cs index 58c5018b2..a22c8db80 100644 --- a/ICSharpCode.Decompiler/CSharp/TranslatedExpression.cs +++ b/ICSharpCode.Decompiler/CSharp/TranslatedExpression.cs @@ -27,7 +27,6 @@ using ICSharpCode.Decompiler.CSharp.Transforms; using ICSharpCode.Decompiler.IL; using ICSharpCode.Decompiler.Semantics; using ICSharpCode.Decompiler.TypeSystem; -using ICSharpCode.Decompiler.TypeSystem.Implementation; using ICSharpCode.Decompiler.Util; #nullable enable @@ -255,7 +254,7 @@ namespace ICSharpCode.Decompiler.CSharp case ConversionResolveResult conversion: { if (Expression is CastExpression cast && CastCanBeMadeImplicit( - Resolver.CSharpConversions.Get(expressionBuilder.compilation), + expressionBuilder, conversion.Conversion, conversion.Input.Type, type, targetType @@ -271,13 +270,10 @@ namespace ICSharpCode.Decompiler.CSharp else if (Expression is ObjectCreateExpression oce && conversion.Conversion.IsMethodGroupConversion && oce.Arguments.Count == 1 && expressionBuilder.settings.UseImplicitMethodGroupConversion) { - // C# 11 caches static method groups. Keep explicit construction when the IL creates a fresh delegate. - if (conversion.Conversion.Method.IsStatic - && conversion.Conversion.Method is not LocalFunctionMethod { IsStaticLocalFunction: false } - && conversion.Conversion.Method.Parameters.Count == type.GetDelegateInvokeMethod()?.Parameters.Count - && expressionBuilder.settings.GetMinimumRequiredVersion() >= LanguageVersion.CSharp11_0 - && expressionBuilder.currentFunction.Kind != ILFunctionKind.ExpressionTree - && !ILInstructions.Any(i => i is CachedDelegate)) + // Preserve explicit construction if a method-group conversion would introduce caching. + // Delegate types containing anonymous types must be inferred instead. + if (expressionBuilder.MethodGroupConversionWouldBeCached(conversion.Conversion) + && (!expressionBuilder.settings.AnonymousTypes || !type.ContainsAnonymousType())) { return this; } @@ -360,7 +356,7 @@ namespace ICSharpCode.Decompiler.CSharp var conversions = Resolver.CSharpConversions.Get(compilation); if (ResolveResult is ConversionResolveResult conv && Expression is CastExpression cast2 && !conv.Conversion.IsUserDefined - && CastCanBeMadeImplicit(conversions, conv.Conversion, conv.Input.Type, type, targetType)) + && CastCanBeMadeImplicit(expressionBuilder, conv.Conversion, conv.Input.Type, type, targetType)) { var unwrapped = Unwrapped(this.UnwrapChild(cast2.Expression)); if (allowImplicitConversion) @@ -690,8 +686,12 @@ namespace ICSharpCode.Decompiler.CSharp /// would have the same semantics as the existing cast from 'inputType' to 'oldTargetType'. /// The existing cast is classified in 'conversion'. /// - bool CastCanBeMadeImplicit(Resolver.CSharpConversions conversions, Conversion conversion, IType inputType, IType oldTargetType, IType newTargetType) + bool CastCanBeMadeImplicit(ExpressionBuilder expressionBuilder, Conversion conversion, IType inputType, IType oldTargetType, IType newTargetType) { + if (conversion.IsMethodGroupConversion && oldTargetType.Kind == TypeKind.Delegate + && !expressionBuilder.settings.UseImplicitMethodGroupConversion) + return false; + var conversions = Resolver.CSharpConversions.Get(expressionBuilder.compilation); if (!conversion.IsImplicit) { // If the cast was required for the old conversion, avoid making it implicit. diff --git a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs index e0bcba5c5..a41ddf145 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs @@ -751,34 +751,30 @@ namespace ICSharpCode.Decompiler.FlowAnalysis protected internal override void VisitNullCoalescingInstruction(NullCoalescingInstruction inst) { - HandleBinaryWithOptionalEvaluation(inst, inst.ValueInst, inst.FallbackInst); + HandleOptionalEvaluation(inst, inst.FallbackInst, precedingArgument: inst.ValueInst); } protected internal override void VisitCachedDelegate(CachedDelegate inst) { - DebugStartPoint(inst); - State cachedState = state.Clone(); - inst.Argument.AcceptVisitor(this); - state.JoinWith(cachedState); - DebugEndPoint(inst); + HandleOptionalEvaluation(inst, inst.Argument); } protected internal override void VisitDynamicLogicOperatorInstruction(DynamicLogicOperatorInstruction inst) { - HandleBinaryWithOptionalEvaluation(inst, inst.Left, inst.Right); + HandleOptionalEvaluation(inst, inst.Right, precedingArgument: inst.Left); } protected internal override void VisitUserDefinedLogicOperator(UserDefinedLogicOperator inst) { - HandleBinaryWithOptionalEvaluation(inst, inst.Left, inst.Right); + HandleOptionalEvaluation(inst, inst.Right, precedingArgument: inst.Left); } - void HandleBinaryWithOptionalEvaluation(ILInstruction parent, ILInstruction left, ILInstruction right) + void HandleOptionalEvaluation(ILInstruction parent, ILInstruction optionalArgument, ILInstruction precedingArgument = null) { DebugStartPoint(parent); - left.AcceptVisitor(this); + precedingArgument?.AcceptVisitor(this); State branchState = state.Clone(); - right.AcceptVisitor(this); + optionalArgument.AcceptVisitor(this); state.JoinWith(branchState); DebugEndPoint(parent); } diff --git a/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs b/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs index 93941cdbe..68581b738 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/CachedDelegateInitialization.cs @@ -33,46 +33,40 @@ namespace ICSharpCode.Decompiler.IL.Transforms return; for (int i = context.IndexOfFirstAlreadyTransformedInstruction - 1; i >= 0; i--) { - if (block.Instructions[i] is IfInstruction inst) + if (block.Instructions[i] is IfInstruction inst && TryTransform(block, i, inst)) { - if (CachedDelegateInitializationWithField(inst)) - { - block.Instructions.RemoveAt(i); - context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count; - continue; - } - if (CachedDelegateInitializationWithLocal(inst)) - { - ILInlining.InlineOneIfPossible(block, i, InliningOptions.Aggressive, context); - context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count; - continue; - } - if (CachedDelegateInitializationRoslynInStaticWithLocal(inst) || CachedDelegateInitializationRoslynWithLocal(inst)) - { - block.Instructions.RemoveAt(i); - context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count; - continue; - } - if (CachedDelegateInitializationVB(inst)) - { - context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count; - continue; - } - if (CachedDelegateInitializationVBWithReturn(inst)) - { - block.Instructions.RemoveAt(i); - context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count; - continue; - } - if (CachedDelegateInitializationVBWithClosure(inst)) - { - context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count; - continue; - } + context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count; } } } + bool TryTransform(Block block, int i, IfInstruction inst) + { + if (CachedDelegateInitializationWithField(inst)) + { + block.Instructions.RemoveAt(i); + return true; + } + if (CachedDelegateInitializationWithLocal(inst)) + { + ILInlining.InlineOneIfPossible(block, i, InliningOptions.Aggressive, context); + return true; + } + if (CachedDelegateInitializationRoslynWithLocal(inst)) + { + block.Instructions.RemoveAt(i); + return true; + } + if (CachedDelegateInitializationVB(inst)) + return true; + if (CachedDelegateInitializationVBWithReturn(inst)) + { + block.Instructions.RemoveAt(i); + return true; + } + return CachedDelegateInitializationVBWithClosure(inst); + } + /// /// if (comp(ldsfld CachedAnonMethodDelegate == ldnull)) { /// stsfld CachedAnonMethodDelegate(DelegateConstruction) @@ -147,38 +141,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms return true; } - /// - /// stloc s(ldobj(ldsflda(CachedAnonMethodDelegate)) - /// if (comp(ldloc s == null)) { - /// stloc s(stobj(ldsflda(CachedAnonMethodDelegate), DelegateConstruction)) - /// } - /// => - /// stloc s(cached.delegate(DelegateConstruction)) - /// - bool CachedDelegateInitializationRoslynInStaticWithLocal(IfInstruction inst) - { - Block trueInst = inst.TrueInst as Block; - if (trueInst == null || (trueInst.Instructions.Count != 1) || !inst.FalseInst.MatchNop()) - return false; - if (!inst.Condition.MatchCompEquals(out ILInstruction left, out ILInstruction right) || !left.MatchLdLoc(out ILVariable s) || !right.MatchLdNull()) - return false; - var storeInst = trueInst.Instructions.Last() as StLoc; - var storeBeforeIf = inst.Parent.Children.ElementAtOrDefault(inst.ChildIndex - 1) as StLoc; - if (storeBeforeIf == null || storeInst == null || storeBeforeIf.Variable != s || storeInst.Variable != s) - return false; - if (!(storeInst.Value is StObj stobj) || !(storeBeforeIf.Value is LdObj ldobj)) - return false; - if (!(stobj.Value is NewObj)) - return false; - if (!stobj.Target.MatchLdsFlda(out var field1) || !ldobj.Target.MatchLdsFlda(out var field2) || !field1.Equals(field2)) - return false; - if (!DelegateConstruction.MatchDelegateConstruction((NewObj)stobj.Value, out _, out _, out _, true)) - return false; - context.Step("CachedDelegateInitializationRoslynInStaticWithLocal", inst); - storeBeforeIf.Value = new CachedDelegate(stobj.Value); - return true; - } - /// /// stloc s(ldobj(ldflda(CachedAnonMethodDelegate)) /// if (comp(ldloc s == null)) { @@ -186,6 +148,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// } /// => /// stloc s(cached.delegate(DelegateConstruction)) + /// The same pattern applies to static cache fields accessed with ldsflda. /// bool CachedDelegateInitializationRoslynWithLocal(IfInstruction inst) { @@ -200,14 +163,19 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; if (!(storeInst.Value is StObj stobj) || !(storeBeforeIf.Value is LdObj ldobj)) return false; - if (!(stobj.Value is NewObj)) + if (stobj.Value is not NewObj delegateConstruction) return false; - if (!stobj.Target.MatchLdFlda(out var _, out var field1) || !ldobj.Target.MatchLdFlda(out var __, out var field2) || !field1.Equals(field2)) + bool sameField = (stobj.Target, ldobj.Target) switch { + (LdsFlda first, LdsFlda second) => first.Field.Equals(second.Field), + (LdFlda first, LdFlda second) => first.Field.Equals(second.Field), + _ => false + }; + if (!sameField) return false; - if (!DelegateConstruction.MatchDelegateConstruction((NewObj)stobj.Value, out _, out _, out _, true)) + if (!DelegateConstruction.MatchDelegateConstruction(delegateConstruction, out _, out _, out _, true)) return false; context.Step("CachedDelegateInitializationRoslynWithLocal", inst); - storeBeforeIf.Value = new CachedDelegate(stobj.Value); + storeBeforeIf.Value = new CachedDelegate(delegateConstruction); return true; } diff --git a/global.json b/global.json index 58a3053ca..115ca7301 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "11.0.100-preview.7.26381.103", + "version": "11.0.0", "rollForward": "major", "allowPrerelease": true },