From bd7f8f9e0901c4f59aafe8933e68220a36de41d4 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Tue, 25 Aug 2026 06:38:22 +0200 Subject: [PATCH 1/4] Let an indexer access leave out arguments and name them HandleAccessorCall had no way to express an omitted argument, so CallBuilder asserted that none had been detected before it got there: any assembly indexing through an indexer with an optional parameter hit that assert in a Debug build, and Release wrote the defaults back out. Accessor calls now go through the same ArgumentList helpers as an ordinary call. Two things had to reach them. The assigned value of a setter is the last argument of the accessor call but not an argument of the access - the standard adds it only for the invocation (12.6.2.1) - so it neither ends the run of optional arguments nor is written out with them, and whether an accessor is written as an access at all is decided once, before the arguments are translated, so the scan and the count cannot disagree. The names have to stop where the argument list does, and they name the indexer's parameters, which the type system takes from the getter rather than from the accessor being called. C# allows named arguments in an element access, but NamedArgumentTransform refused to introduce one for any accessor, so an access whose arguments the compiler reordered came out as a temporary. Only indexers gain this: a property access has no argument list, an operator cannot take names, and a setter's value stays unnamed on the right-hand side. Introducing a name replaces the call with a block, so it is refused where the surrounding instruction requires the call itself - a call-inline-assign block, or the target of a compound assignment. Whether the shortened access still binds to the same member is left to IsUnambiguousAccess. If it does not, the omitted arguments are written out again before any cast is tried, since restoring them cannot change what the access means. A type declaring both this[int] and this[int, int = 10] therefore keeps both arguments; the fixture pins that. Not covered: params indexers, [Optional] without a constant, [DateTimeConstant]-style defaults, default(T) at a value-type instantiation, and omitting a middle optional argument - the last of which plain calls do not do either. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- .../ICSharpCode.Decompiler.Tests.csproj | 8 + .../ILPrettyTestRunner.cs | 24 +++ .../Correctness/OverloadResolution.cs | 31 ++++ .../ILPretty/IndexerAccessorParameterNames.cs | 20 +++ .../ILPretty/IndexerAccessorParameterNames.il | 70 ++++++++ .../ParameterizedPropertyInitializer.cs | 21 +++ .../ParameterizedPropertyInitializer.il | 60 +++++++ .../ParameterizedPropertySetterCall.cs | 19 ++ .../ParameterizedPropertySetterCall.il | 55 ++++++ .../ILPretty/ParamsPropertySetter.cs | 18 ++ .../ILPretty/ParamsPropertySetter.il | 68 ++++++++ .../TestCases/Pretty/NamedArguments.cs | 62 +++++++ .../TestCases/Pretty/OptionalArguments.cs | 125 +++++++++++++ .../Pretty/OptionalArgumentsDisabled.cs | 16 ++ ICSharpCode.Decompiler/CSharp/CallBuilder.cs | 164 +++++++++++++----- .../IL/Transforms/NamedArgumentTransform.cs | 52 +++++- 16 files changed, 763 insertions(+), 50 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/ILPretty/IndexerAccessorParameterNames.cs create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/ILPretty/IndexerAccessorParameterNames.il create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertyInitializer.cs create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertyInitializer.il create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertySetterCall.cs create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertySetterCall.il create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParamsPropertySetter.cs create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParamsPropertySetter.il diff --git a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj index 714454f75..d09ee4225 100644 --- a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj @@ -218,6 +218,14 @@ + + + + + + + + diff --git a/ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs index 7e9f8df94..70cf2bc8c 100644 --- a/ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs @@ -429,6 +429,30 @@ namespace ICSharpCode.Decompiler.Tests await Run(); } + [Test] + public async Task ParamsPropertySetter() + { + await Run(); + } + + [Test] + public async Task ParameterizedPropertyInitializer() + { + await Run(); + } + + [Test] + public async Task IndexerAccessorParameterNames() + { + await Run(); + } + + [Test] + public async Task ParameterizedPropertySetterCall() + { + await Run(); + } + async Task Run([CallerMemberName] string testName = null, DecompilerSettings settings = null, AssemblerOptions assemblerOptions = AssemblerOptions.Library) { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/OverloadResolution.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/OverloadResolution.cs index c8107d1ec..412d61282 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/OverloadResolution.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/OverloadResolution.cs @@ -32,6 +32,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness Generics(); ConstructorTest(); TestIndexer(); + TestIndexerWithNamedArguments(); Issue1281(); Issue1747(); CallAmbiguousOutParam(); @@ -330,6 +331,23 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness } #endregion + #region Indexer with named arguments + static void TestIndexerWithNamedArguments() + { + var obj = new NamedArgumentIndexerTests(); + Console.WriteLine(obj[y: Trace(1), x: Trace(2)]); + obj[y: Trace(3), x: Trace(4)] = Trace(5); + Console.WriteLine(obj[y: Trace(6), x: Trace(7)] = Trace(8)); + obj[y: Trace(9), x: Trace(10)] += 5; + } + + static int Trace(int i) + { + Console.WriteLine("Trace(" + i + ")"); + return i; + } + #endregion + #region Out Parameter static void AmbiguousOutParam(out string a) { @@ -602,6 +620,19 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness #endregion } + class NamedArgumentIndexerTests + { + public int this[int x, int y] { + get { + Console.WriteLine("get_Item(" + x + ", " + y + ")"); + return x; + } + set { + Console.WriteLine("set_Item(" + x + ", " + y + ", " + value + ")"); + } + } + } + class IndexerTests { public object this[object key] { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/IndexerAccessorParameterNames.cs b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/IndexerAccessorParameterNames.cs new file mode 100644 index 000000000..ae421c420 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/IndexerAccessorParameterNames.cs @@ -0,0 +1,20 @@ +public class IndexerAccessorParameterNames +{ + public int this[int x, int y] { + get { + return x; + } + set { + } + } + + private int Get(int i) + { + return i; + } + + public void Use() + { + this[y: Get(1), x: Get(2)] = 3; + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/IndexerAccessorParameterNames.il b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/IndexerAccessorParameterNames.il new file mode 100644 index 000000000..6a4ad53ba --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/IndexerAccessorParameterNames.il @@ -0,0 +1,70 @@ +#define CORE_ASSEMBLY "System.Runtime" + +.assembly extern CORE_ASSEMBLY +{ + .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....: + .ver 4:0:0:0 +} + +.assembly IndexerAccessorParameterNames { } + +.class public auto ansi beforefieldinit IndexerAccessorParameterNames + extends [CORE_ASSEMBLY]System.Object +{ + .custom instance void [CORE_ASSEMBLY]System.Reflection.DefaultMemberAttribute::.ctor(string) = ( 01 00 04 49 74 65 6D 00 00 ) + + // The indexer's parameter names are the getter's: x and y. + .method public hidebysig specialname instance int32 get_Item (int32 x, int32 y) cil managed + { + .maxstack 8 + ldarg.1 + ret + } + + // The setter is free to name the same parameters differently, which C# cannot express. + .method public hidebysig specialname instance void set_Item (int32 a, int32 b, int32 'value') cil managed + { + .maxstack 8 + ret + } + + .property instance int32 Item(int32, int32) + { + .get instance int32 IndexerAccessorParameterNames::get_Item(int32, int32) + .set instance void IndexerAccessorParameterNames::set_Item(int32, int32, int32) + } + + .method private hidebysig instance int32 Get (int32 i) cil managed + { + .maxstack 8 + ldarg.1 + ret + } + + // this[y: Get(1), x: Get(2)] = 3; + .method public hidebysig instance void Use () cil managed + { + .maxstack 4 + .locals init (int32 V_0) + ldarg.0 + ldarg.0 + ldc.i4.1 + call instance int32 IndexerAccessorParameterNames::Get(int32) + stloc.0 + ldarg.0 + ldc.i4.2 + call instance int32 IndexerAccessorParameterNames::Get(int32) + ldloc.0 + ldc.i4.3 + call instance void IndexerAccessorParameterNames::set_Item(int32, int32, int32) + ret + } + + .method public hidebysig specialname rtspecialname instance void .ctor () cil managed + { + .maxstack 8 + ldarg.0 + call instance void [CORE_ASSEMBLY]System.Object::.ctor() + ret + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertyInitializer.cs b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertyInitializer.cs new file mode 100644 index 000000000..0b1347b34 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertyInitializer.cs @@ -0,0 +1,21 @@ +public class ParameterizedPropertyInitializer +{ + // C# has no syntax for parameterized property 'Foo'. + public int get_Foo(int x) + { + return x; + } + + public void set_Foo(int x, int value) + { + } + + public static void Consume(ParameterizedPropertyInitializer p) + { + } + + public static void Use() + { + Consume(new ParameterizedPropertyInitializer { [7] = 5 }); + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertyInitializer.il b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertyInitializer.il new file mode 100644 index 000000000..38a835abd --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertyInitializer.il @@ -0,0 +1,60 @@ +#define CORE_ASSEMBLY "System.Runtime" + +.assembly extern CORE_ASSEMBLY +{ + .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....: + .ver 4:0:0:0 +} + +.assembly ParameterizedPropertyInitializer { } + +// A parameterized property that is not an indexer: the type carries no DefaultMemberAttribute, +// which C# cannot express, but VB, C++/CLI and COM interop all produce it. +.class public auto ansi beforefieldinit ParameterizedPropertyInitializer + extends [CORE_ASSEMBLY]System.Object +{ + .method public hidebysig specialname instance int32 get_Foo (int32 x) cil managed + { + .maxstack 8 + ldarg.1 + ret + } + + .method public hidebysig specialname instance void set_Foo (int32 x, int32 'value') cil managed + { + .maxstack 8 + ret + } + + .property instance int32 Foo(int32) + { + .get instance int32 ParameterizedPropertyInitializer::get_Foo(int32) + .set instance void ParameterizedPropertyInitializer::set_Foo(int32, int32) + } + + .method public hidebysig static void Consume (class ParameterizedPropertyInitializer p) cil managed + { + .maxstack 8 + ret + } + + .method public hidebysig static void Use () cil managed + { + .maxstack 8 + newobj instance void ParameterizedPropertyInitializer::.ctor() + dup + ldc.i4.7 + ldc.i4.5 + call instance void ParameterizedPropertyInitializer::set_Foo(int32, int32) + call void ParameterizedPropertyInitializer::Consume(class ParameterizedPropertyInitializer) + ret + } + + .method public hidebysig specialname rtspecialname instance void .ctor () cil managed + { + .maxstack 8 + ldarg.0 + call instance void [CORE_ASSEMBLY]System.Object::.ctor() + ret + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertySetterCall.cs b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertySetterCall.cs new file mode 100644 index 000000000..8167f578f --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertySetterCall.cs @@ -0,0 +1,19 @@ +using System.Runtime.InteropServices; + +public class ParameterizedPropertySetterCall +{ + // C# has no syntax for parameterized property 'P'. + public int get_P(int i) + { + return i; + } + + public void set_P([Optional][DefaultParameterValue(0)] int i, int value) + { + } + + public void Use() + { + this.set_P(0, 5); + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertySetterCall.il b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertySetterCall.il new file mode 100644 index 000000000..0582a2dea --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParameterizedPropertySetterCall.il @@ -0,0 +1,55 @@ +#define CORE_ASSEMBLY "System.Runtime" + +.assembly extern CORE_ASSEMBLY +{ + .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....: + .ver 4:0:0:0 +} + +.assembly ParameterizedPropertySetterCall { } + +// A parameterized property whose setter takes an index and the assigned value. It is not the +// type's default member, so there is no access syntax for it and the accessor is written as a +// call - which means the assigned value is an ordinary argument, and the optional index before +// it is not trailing. +.class public auto ansi beforefieldinit ParameterizedPropertySetterCall + extends [CORE_ASSEMBLY]System.Object +{ + .method public hidebysig specialname instance int32 get_P (int32 i) cil managed + { + .maxstack 8 + ldarg.1 + ret + } + + .method public hidebysig specialname instance void set_P ([opt] int32 i, int32 'value') cil managed + { + .param [1] = int32(0x00000000) + .maxstack 8 + ret + } + + .property instance int32 P(int32) + { + .get instance int32 ParameterizedPropertySetterCall::get_P(int32) + .set instance void ParameterizedPropertySetterCall::set_P(int32, int32) + } + + .method public hidebysig instance void Use () cil managed + { + .maxstack 8 + ldarg.0 + ldc.i4.0 + ldc.i4.5 + call instance void ParameterizedPropertySetterCall::set_P(int32, int32) + ret + } + + .method public hidebysig specialname rtspecialname instance void .ctor () cil managed + { + .maxstack 8 + ldarg.0 + call instance void [CORE_ASSEMBLY]System.Object::.ctor() + ret + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParamsPropertySetter.cs b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParamsPropertySetter.cs new file mode 100644 index 000000000..7149658d3 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParamsPropertySetter.cs @@ -0,0 +1,18 @@ +public class ParamsPropertySetter +{ + private int[] values; + + public int[] Values { + get { + return values; + } + set { + values = value; + } + } + + public void Use() + { + Values = new int[2] { 1, 2 }; + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParamsPropertySetter.il b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParamsPropertySetter.il new file mode 100644 index 000000000..10fbb2bff --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ParamsPropertySetter.il @@ -0,0 +1,68 @@ +#define CORE_ASSEMBLY "System.Runtime" + +.assembly extern CORE_ASSEMBLY +{ + .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....: + .ver 4:0:0:0 +} + +.assembly ParamsPropertySetter { } + +.class public auto ansi beforefieldinit ParamsPropertySetter + extends [CORE_ASSEMBLY]System.Object +{ + .field private int32[] 'values' + + .method public hidebysig specialname instance int32[] get_Values () cil managed + { + .maxstack 8 + ldarg.0 + ldfld int32[] ParamsPropertySetter::'values' + ret + } + + // C# cannot declare a property whose value is a parameter array, and an assignment has no + // argument list to expand one into. + .method public hidebysig specialname instance void set_Values (int32[] 'value') cil managed + { + .param [1] + .custom instance void [CORE_ASSEMBLY]System.ParamArrayAttribute::.ctor() = ( 01 00 00 00 ) + .maxstack 8 + ldarg.0 + ldarg.1 + stfld int32[] ParamsPropertySetter::'values' + ret + } + + .property instance int32[] Values() + { + .get instance int32[] ParamsPropertySetter::get_Values() + .set instance void ParamsPropertySetter::set_Values(int32[]) + } + + .method public hidebysig instance void Use () cil managed + { + .maxstack 4 + ldarg.0 + ldc.i4.2 + newarr [CORE_ASSEMBLY]System.Int32 + dup + ldc.i4.0 + ldc.i4.1 + stelem.i4 + dup + ldc.i4.1 + ldc.i4.2 + stelem.i4 + call instance void ParamsPropertySetter::set_Values(int32[]) + ret + } + + .method public hidebysig specialname rtspecialname instance void .ctor () cil managed + { + .maxstack 8 + ldarg.0 + call instance void [CORE_ASSEMBLY]System.Object::.ctor() + ret + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NamedArguments.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NamedArguments.cs index 293d23e04..4fed08cde 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NamedArguments.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NamedArguments.cs @@ -60,6 +60,52 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty } } + public class BaseNames + { + public virtual int this[int x, int y] { + get { + return x; + } + set { + } + } + } + + public class DerivedNames : BaseNames + { + public override int this[int a, int b] { + get { + return a; + } + set { + } + } + } + + public int this[int x, int y] { + get { + return x; + } + set { + } + } + + public int this[int i, object o] { + get { + return i; + } + set { + } + } + + public int this[int i, string o] { + get { + return i; + } + set { + } + } + public void Use(int a, int b, int c) { } @@ -81,5 +127,21 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty int b = Get(1); Use(Get(2), b, Get(3)); } + + public void NamedArgsForIndexer() + { + Use(this[y: Get(1), x: Get(2)], 0, 0); + this[y: Get(1), x: Get(2)] = 3; + } + + public void NamedArgsForIndexerNeedingCast() + { + Use(this[o: (object)((Get(1) == 1) ? "a" : "b"), i: Get(2)], 0, 0); + } + public void NamedArgsForOverriddenIndexer(DerivedNames derived) + { + // The names are the base indexer's, which is what the call instruction names. + Use(((BaseNames)derived)[y: Get(1), x: Get(2)], 0, 0); + } } } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArguments.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArguments.cs index b45e349b7..73ec58395 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArguments.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArguments.cs @@ -55,6 +55,83 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty } } + internal class Indexer + { + public int this[int x, int y = 10] { + get { + return x + y; + } + set { + } + } + } + + internal class IndexerWithOverload + { + public int this[int x] { + get { + return x; + } + set { + } + } + + public int this[int x, int y = 10] { + get { + return x + y; + } + set { + } + } + } + + internal class AllOptionalIndexer + { + public int this[int x = 10, int y = 20] { + get { + return x + y; + } + set { + } + } + } + + internal class BaseIndexer + { + public virtual int this[bool flag] { + get { + return 1; + } + set { + } + } + } + + internal class DerivedIndexer : BaseIndexer + { + public override int this[bool f] { + get { + return 2; + } + set { + } + } + } + + [StructLayout(LayoutKind.Sequential, Size = 1)] + internal struct StructIndexer + { + public int this[int x, int y = 10] { + get { + return x + y; + } + set { + } + } + } + + private static StructIndexer structIndexer; + public OptionalArguments(string name, int a = 5) { @@ -330,5 +407,53 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty d(42); } #endif + + private void Indexers(Indexer indexer, IndexerWithOverload overloaded) + { + Console.WriteLine(indexer[1]); + Console.WriteLine(indexer[1, 20]); + indexer[1] = 5; + indexer[1] += 5; + indexer[1]++; + Console.WriteLine(structIndexer[1]); + structIndexer[1] = 5; + // Leaving the argument out would bind to the single-parameter indexer. + Console.WriteLine(overloaded[1, 10]); + Console.WriteLine(overloaded[1]); + } + + private void AllOptionalIndexers(AllOptionalIndexer allOptional, BaseIndexer boolIndexer, DerivedIndexer derived) + { + // An indexer access keeps an argument even when every one of them is optional. + Console.WriteLine(allOptional[10]); + allOptional[10] = 5; + // Primitive values are not named in an access, unlike in a call. The second one binds + // to an override that names the parameter differently. + Console.WriteLine(boolIndexer[true]); + Console.WriteLine(derived[true]); + } + + // Only the index initializers below need C# 6. +#if CS60 + private Indexer IndexerInitializer() + { + return new Indexer { + [1] = 5, + [2, 20] = 6 + }; + } + + private BaseIndexer BoolIndexerInitializer() + { + // An index initializer does not name its arguments either. + return new BaseIndexer { [true] = 5 }; + } + + private DerivedIndexer DerivedIndexerInitializer() + { + // The call goes to the base indexer, which names the parameter differently. + return new DerivedIndexer { [true] = 7 }; + } +#endif } } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArgumentsDisabled.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArgumentsDisabled.cs index b33456f2c..5a87ea695 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArgumentsDisabled.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArgumentsDisabled.cs @@ -1,7 +1,17 @@ +using System; + namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty { public class OptionalArgumentsDisabled { + public int this[int x, int y = 10] { + get { + return x + y; + } + set { + } + } + public void Test() { MixedArguments("123", 0, 0); @@ -15,5 +25,11 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty public void OnlyOptionalArguments(int a = 0, int b = 0) { } + + public void TestIndexer() + { + Console.WriteLine(this[1, 10]); + this[1, 10] = 5; + } } } diff --git a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs index d4cb8e090..c35e0e50a 100644 --- a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs @@ -58,12 +58,15 @@ namespace ICSharpCode.Decompiler.CSharp public bool AddNamesToPrimitiveValues; public bool UseImplicitlyTypedOut; public bool IsExpandedForm; + public bool IsSetter; public int Length => Arguments.Length; - private int GetActualArgumentCount() + public int GetActualArgumentCount() { + int count = IsSetter ? Arguments.Length - 1 : Arguments.Length; if (FirstOptionalArgumentIndex < 0) - return Arguments.Length; + return count; + Debug.Assert(FirstOptionalArgumentIndex <= count); return FirstOptionalArgumentIndex; } @@ -88,10 +91,19 @@ namespace ICSharpCode.Decompiler.CSharp } } + // The names cover the full parameter list and have to stop where the arguments do. + int argumentCount = GetActualArgumentCount(); + if (argumentNames != null && argumentNames.Length > argumentCount) + { + var writtenNames = new string[argumentCount]; + Array.Copy(argumentNames, writtenNames, argumentCount); + argumentNames = writtenNames; + } + return argumentNames; } - public IList GetArgumentResolveResults(int skipCount = 0) + public ResolveResult[] GetArgumentResolveResults(int skipCount = 0) { var expectedParameters = ExpectedParameters; var useImplicitlyTypedOut = UseImplicitlyTypedOut; @@ -132,7 +144,7 @@ namespace ICSharpCode.Decompiler.CSharp else { Debug.Assert(skipCount == 0); - return Arguments.Take(argumentCount).Zip(argumentNames.Take(argumentCount), + return Arguments.Take(argumentCount).Zip(argumentNames, (arg, name) => { if (name == null) return AddAnnotations(arg.Expression); @@ -500,11 +512,17 @@ namespace ICSharpCode.Decompiler.CSharp return result; } - int allowedParamCount = (method.ReturnType.IsKnownType(KnownTypeCode.Void) ? 1 : 0); - if (method.IsAccessor && (method.AccessorOwner.SymbolKind == SymbolKind.Indexer || argumentList.ExpectedParameters.Length == allowedParamCount)) + // IsSetter carries the answer for an accessor that takes the assigned value, including + // the argument order that keeps it last. + if (argumentList.IsSetter || (!TakesAssignedValueLast(method) && IsWrittenAsMemberAccess(method))) { - argumentList.CheckNoNamedOrOptionalArguments(); - return HandleAccessorCall(expectedTargetDetails, method, target, argumentList.Arguments.ToList(), argumentList.ArgumentNames); + // Only an indexer access has an argument list to carry names or leave arguments out of. + if (method.AccessorOwner!.SymbolKind != SymbolKind.Indexer) + argumentList.CheckNoNamedOrOptionalArguments(); + // An access spells its index out anyway, and the ladder answers a name the member + // does not have with a cast of the target rather than by giving the name up. + argumentList.AddNamesToPrimitiveValues = false; + return HandleAccessorCall(expectedTargetDetails, method, target, argumentList); } if (IsDelegateEqualityComparison(method, argumentList.Arguments)) @@ -793,10 +811,15 @@ namespace ICSharpCode.Decompiler.CSharp callArguments.Add(value ?? new Nop()); var argumentList = BuildArgumentList(expectedTargetDetails, target, method, 1, callArguments, null); + // An index initializer is an assignment whatever the accessor looks like, even for a + // parameterized property, which has no access syntax of its own. + argumentList.IsSetter = true; + // The cast the ladder would answer an unresolvable name with is removed again below, + // together with the target. + argumentList.AddNamesToPrimitiveValues = false; var unused = new IdentifierExpression("initializedObject").WithRR(target).WithoutILInstruction(); - var assignment = HandleAccessorCall(expectedTargetDetails, method, unused, - argumentList.Arguments.ToList(), argumentList.ArgumentNames); + var assignment = HandleAccessorCall(expectedTargetDetails, method, unused, argumentList); if (((AssignmentExpression)assignment).Left is IndexerExpression indexer && indexer.Target is not null) indexer.Target.Remove(); @@ -1013,6 +1036,17 @@ namespace ICSharpCode.Decompiler.CSharp // >= 0 - the index of the first argument that can be removed, because it is optional // and is the default value of the parameter. int firstOptionalArgumentIndex = expressionBuilder.settings.OptionalArguments ? -2 : -1; + // Only an accessor written as an access takes its assigned value out of the argument + // list; one written as a call passes it like any other argument. The scan below and + // GetActualArgumentCount() have to agree on that, so it is decided once, here. + bool writtenAsMemberAccess = IsWrittenAsMemberAccess(method); + bool isSetter = writtenAsMemberAccess && TakesAssignedValueLast(method); + // A named argument of an indexer access names a parameter of the indexer, which the type + // system takes from the getter. The accessor being called may name the same parameters + // differently - C# cannot declare that, but other languages can. + IReadOnlyList namedParameters = method.AccessorOwner is IProperty { IsIndexer: true } indexer + ? indexer.Parameters + : method.Parameters; for (int i = firstParamIndex; i < callArguments.Count; i++) { IParameter parameter; @@ -1024,10 +1058,13 @@ namespace ICSharpCode.Decompiler.CSharp // assign names to that argument and all following arguments: argumentNames = new string[method.Parameters.Count]; } - parameter = method.Parameters[argumentToParameterMap[i]]; - if (argumentNames != null && AssignVariableNames.IsValidName(parameter.Name)) + int parameterIndex = argumentToParameterMap[i]; + parameter = method.Parameters[parameterIndex]; + // The assigned value is past the end of the indexer's parameters. + if (argumentNames != null && parameterIndex < namedParameters.Count + && AssignVariableNames.IsValidName(namedParameters[parameterIndex].Name)) { - argumentNames[arguments.Count] = parameter.Name; + argumentNames[arguments.Count] = namedParameters[parameterIndex].Name; } } else @@ -1039,17 +1076,24 @@ namespace ICSharpCode.Decompiler.CSharp { isPrimitiveValue.Set(arguments.Count); } - if (IsOptionalArgument(parameter, arg)) + // The assigned value of a setter is not part of the argument list, so it does not + // end the run of optional arguments either. + if (!(isSetter && i + 1 == callArguments.Count)) { - if (firstOptionalArgumentIndex == -2) - firstOptionalArgumentIndex = i - firstParamIndex; - } - else - { - if (firstOptionalArgumentIndex != -1) + if (IsOptionalArgument(parameter, arg)) + { + if (firstOptionalArgumentIndex == -2) + firstOptionalArgumentIndex = i - firstParamIndex; + } + else if (firstOptionalArgumentIndex != -1) + { firstOptionalArgumentIndex = -2; + } } - if (expressionBuilder.settings.ExpandParamsArguments && parameter.IsParams && i + 1 == callArguments.Count && argumentToParameterMap == null) + // An assignment has no argument list to spread a parameter array over, and C# + // cannot declare a property whose value is one. + if (expressionBuilder.settings.ExpandParamsArguments && parameter.IsParams && !isSetter + && i + 1 == callArguments.Count && argumentToParameterMap == null) { // Parameter is marked params // If the argument is an array creation, inline all elements into the call and add missing default values. @@ -1111,6 +1155,7 @@ namespace ICSharpCode.Decompiler.CSharp list.IsExpandedForm = isExpandedForm; list.IsPrimitiveValue = isPrimitiveValue; list.FirstOptionalArgumentIndex = firstOptionalArgumentIndex; + list.IsSetter = isSetter; list.UseImplicitlyTypedOut = true; list.AddNamesToPrimitiveValues = expressionBuilder.settings.NamedArguments && expressionBuilder.settings.NonTrailingNamedArguments; return list; @@ -1133,8 +1178,7 @@ namespace ICSharpCode.Decompiler.CSharp expandedParameters.InsertRange(0, expectedParameters); expandedArguments.InsertRange(0, arguments); if (IsUnambiguousCall(expectedTargetDetails, method, targetResolveResult, Empty.Array, - expandedArguments.SelectArray(a => a.ResolveResult), argumentNames: null, - firstOptionalArgumentIndex: -1, out _, + expandedArguments.SelectArray(a => a.ResolveResult), argumentNames: null, out _, out var bestCandidateIsExpandedForm) == OverloadResolutionErrors.None && bestCandidateIsExpandedForm) { expectedParameters = expandedParameters; @@ -1309,7 +1353,7 @@ namespace ICSharpCode.Decompiler.CSharp bool skipTargetCast = method.Accessibility <= Accessibility.Protected && expressionBuilder.IsBaseTypeOfCurrentType(method.DeclaringTypeDefinition); OverloadResolutionErrors errors; while ((errors = IsUnambiguousCall(expectedTargetDetails, method, targetResolveResult, typeArguments, - argumentList.GetArgumentResolveResults().ToArray(), argumentList.GetArgumentNames(), argumentList.FirstOptionalArgumentIndex, out foundMethod, + argumentList.GetArgumentResolveResults().ToArray(), argumentList.GetArgumentNames(), out foundMethod, out var bestCandidateIsExpandedForm)) != OverloadResolutionErrors.None || bestCandidateIsExpandedForm != argumentList.IsExpandedForm) { switch (errors) @@ -1634,7 +1678,7 @@ namespace ICSharpCode.Decompiler.CSharp OverloadResolutionErrors IsUnambiguousCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, ResolveResult? target, IType[] typeArguments, ResolveResult[] arguments, - string[]? argumentNames, int firstOptionalArgumentIndex, + string[]? argumentNames, out IParameterizedMember? foundMember, out bool bestCandidateIsExpandedForm) { foundMember = null; @@ -1645,10 +1689,6 @@ namespace ICSharpCode.Decompiler.CSharp Log.WriteLine("IsUnambiguousCall: Performing overload resolution for " + method); Log.WriteCollection(" Arguments: ", arguments); - argumentNames = firstOptionalArgumentIndex < 0 || argumentNames == null - ? argumentNames - : argumentNames.Take(firstOptionalArgumentIndex).ToArray(); - var or = new OverloadResolution(resolver.Compilation, arguments, argumentNames, typeArguments, conversions: expressionBuilder.resolver.conversions); @@ -1751,10 +1791,10 @@ namespace ICSharpCode.Decompiler.CSharp } bool IsUnambiguousAccess(ExpectedTargetDetails expectedTargetDetails, ResolveResult? target, IMethod method, - IList arguments, string[]? argumentNames, [NotNullWhen(true)] out IMember? foundMember) + IList arguments, string[]? argumentNames, [NotNullWhen(true)] out IMember? foundMember) { Log.WriteLine("IsUnambiguousAccess: Performing overload resolution for " + method); - Log.WriteCollection(" Arguments: ", arguments.Select(a => a.ResolveResult)); + Log.WriteCollection(" Arguments: ", arguments); foundMember = null; if (target == null) @@ -1772,7 +1812,7 @@ namespace ICSharpCode.Decompiler.CSharp if (method.AccessorOwner!.SymbolKind == SymbolKind.Indexer) { var or = new OverloadResolution(resolver.Compilation, - arguments.SelectArray(a => a.ResolveResult), + arguments.ToArray(), argumentNames: argumentNames, typeArguments: Empty.Array, conversions: expressionBuilder.resolver.conversions); @@ -1797,8 +1837,32 @@ namespace ICSharpCode.Decompiler.CSharp return foundMember != null && IsAppropriateCallTarget(expectedTargetDetails, method.AccessorOwner, foundMember); } + /// + /// Whether the accessor's last parameter is the assigned value: a setter takes it, and so do + /// the two event accessors, written as += and -=. + /// + static bool TakesAssignedValueLast(IMethod method) + { + return method.AccessorKind is System.Reflection.MethodSemanticsAttributes.Setter + or System.Reflection.MethodSemanticsAttributes.Adder + or System.Reflection.MethodSemanticsAttributes.Remover; + } + + /// + /// Whether the accessor is written as a property or indexer access. One with more parameters + /// than the access syntax has room for is written as a call, assigned value and all. + /// + static bool IsWrittenAsMemberAccess(IMethod method) + { + if (!method.IsAccessor) + return false; + if (method.AccessorOwner!.SymbolKind == SymbolKind.Indexer) + return true; + return method.Parameters.Count == (TakesAssignedValueLast(method) ? 1 : 0); + } + ExpressionWithResolveResult HandleAccessorCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, - TranslatedExpression target, List arguments, string[]? argumentNames) + TranslatedExpression target, ArgumentList argumentList) { bool requireTarget; if (settings.AlwaysQualifyMemberReferences || method.AccessorOwner!.SymbolKind == SymbolKind.Indexer || expressionBuilder.HidesVariableWithName(method.AccessorOwner.Name)) @@ -1808,24 +1872,32 @@ namespace ICSharpCode.Decompiler.CSharp else requireTarget = !(target.Expression is ThisReferenceExpression); bool targetCasted = false; - bool isSetter = method.ReturnType.IsKnownType(KnownTypeCode.Void); + bool isSetter = argumentList.IsSetter; bool argumentsCasted = (isSetter && method.Parameters.Count == 1) || (!isSetter && method.Parameters.Count == 0); var targetResolveResult = requireTarget ? target.ResolveResult : null; - TranslatedExpression value = default(TranslatedExpression); - if (isSetter) + // Dropping every argument would turn an indexer access into a property access. + if (argumentList.FirstOptionalArgumentIndex == 0 && method.AccessorOwner!.SymbolKind == SymbolKind.Indexer) { - value = arguments.Last(); - arguments.Remove(value); + argumentList.FirstOptionalArgumentIndex = 1; } IMember? foundMember; - while (!IsUnambiguousAccess(expectedTargetDetails, targetResolveResult, method, arguments, argumentNames, out foundMember)) + while (!IsUnambiguousAccess(expectedTargetDetails, targetResolveResult, method, + argumentList.GetArgumentResolveResults(), argumentList.GetArgumentNames(), out foundMember)) { - if (!argumentsCasted) + if (argumentList.FirstOptionalArgumentIndex >= 0) + { + // Unlike the casts below, writing the omitted arguments out again cannot change + // what the access means, so try that first. + argumentList.FirstOptionalArgumentIndex = -1; + } + else if (!argumentsCasted) { argumentsCasted = true; - CastArguments(arguments, method.Parameters.ToList()); + CastArguments( + new ArraySegment(argumentList.Arguments, 0, argumentList.GetActualArgumentCount()), + argumentList.ExpectedParameters); } else if (!requireTarget) { @@ -1845,6 +1917,10 @@ namespace ICSharpCode.Decompiler.CSharp } } + var arguments = argumentList.GetArgumentExpressions().ToList(); + // The assigned value is not one of the arguments the ladder casts, so nothing it could + // try makes an access resolve that fails over the value's type. + TranslatedExpression value = isSetter ? argumentList.Arguments[argumentList.Length - 1] : default; var rr = new MemberResolveResult(target.ResolveResult, foundMember); if (isSetter) @@ -1853,7 +1929,7 @@ namespace ICSharpCode.Decompiler.CSharp if (arguments.Count != 0) { - expr = new IndexerExpression(target.ResolveResult is InitializedObjectResolveResult ? null : target.Expression, arguments.Select(a => a.Expression)) + expr = new IndexerExpression(target.ResolveResult is InitializedObjectResolveResult ? null : target.Expression, arguments) .WithoutILInstruction().WithRR(rr); } else if (requireTarget) @@ -1885,7 +1961,7 @@ namespace ICSharpCode.Decompiler.CSharp { if (arguments.Count != 0) { - return new IndexerExpression(target.Expression, arguments.Select(a => a.Expression)) + return new IndexerExpression(target.Expression, arguments) .WithoutILInstruction().WithRR(rr); } else if (requireTarget) @@ -1967,7 +2043,7 @@ namespace ICSharpCode.Decompiler.CSharp { while (IsUnambiguousCall(expectedTargetDetails, method, null, Empty.Array, argumentList.GetArgumentResolveResults().ToArray(), - argumentList.GetArgumentNames(), argumentList.FirstOptionalArgumentIndex, out _, + argumentList.GetArgumentNames(), out _, out var bestCandidateIsExpandedForm) != OverloadResolutionErrors.None || bestCandidateIsExpandedForm != argumentList.IsExpandedForm) { if (argumentList.AddNamesToPrimitiveValues) diff --git a/ICSharpCode.Decompiler/IL/Transforms/NamedArgumentTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NamedArgumentTransform.cs index 4c37a4afd..1cd87d871 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NamedArgumentTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NamedArgumentTransform.cs @@ -28,13 +28,48 @@ namespace ICSharpCode.Decompiler.IL.Transforms public class NamedArgumentTransform : IStatementTransform { + /// + /// How many arguments may carry a name: a setter's last one is the assigned value, which is + /// written as the right-hand side. + /// + static int NameableArgumentCount(CallInstruction call) + { + if (call.Method.AccessorKind is System.Reflection.MethodSemanticsAttributes.Setter + or System.Reflection.MethodSemanticsAttributes.Adder + or System.Reflection.MethodSemanticsAttributes.Remover) + { + return call.Arguments.Count - 1; + } + return call.Arguments.Count; + } + internal static FindResult CanIntroduceNamedArgument(CallInstruction call, ILInstruction child, ILVariable v, ILInstruction expressionBeingMoved) { Debug.Assert(child.Parent == call); if (call.IsInstanceCall && child.ChildIndex == 0) return FindResult.Stop; // cannot use named arg to move expressionBeingMoved before this pointer - if (call.Method.IsOperator || call.Method.IsAccessor) - return FindResult.Stop; // cannot use named arg for operators or accessors + if (call.Method.IsOperator) + return FindResult.Stop; // cannot use named arg for operators + bool isIndexerSetter = false; + if (call.Method.IsAccessor) + { + // Only an indexer access has an argument list that can carry names. + if (call.Method.AccessorOwner!.SymbolKind != SymbolKind.Indexer) + return FindResult.Stop; + // A name replaces the call with a block: a call-inline-assign block is matched by + // the call it holds, and a compound assignment requires a call in its target. + if (call.Parent is Block { Kind: BlockKind.CallInlineAssign }) + return FindResult.Stop; + if (call.Parent is CompoundAssignmentInstruction { TargetKind: CompoundTargetKind.Property } compoundAssignment + && compoundAssignment.Target == call) + { + return FindResult.Stop; + } + // A setter's last argument is the assigned value, written as the right-hand side. + isIndexerSetter = call.Method.AccessorKind == System.Reflection.MethodSemanticsAttributes.Setter; + if (isIndexerSetter && child.ChildIndex == call.Arguments.Count - 1) + return FindResult.Stop; + } if (call.Method is VarArgInstanceMethod) return FindResult.Stop; // CallBuilder doesn't support named args when using varargs if (call.Method.IsConstructor) @@ -45,7 +80,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms } if (call.Method.Parameters.Any(p => string.IsNullOrEmpty(p.Name))) return FindResult.Stop; // cannot use named arguments - for (int i = child.ChildIndex; i < call.Arguments.Count; i++) + int nameableArgumentCount = isIndexerSetter ? call.Arguments.Count - 1 : call.Arguments.Count; + Debug.Assert(nameableArgumentCount == NameableArgumentCount(call)); + for (int i = child.ChildIndex; i < nameableArgumentCount; i++) { var r = ILInlining.FindLoadInNext(call.Arguments[i], v, expressionBeingMoved, InliningOptions.None); if (r.Type == FindResultType.Found) @@ -87,11 +124,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } } - foreach (var arg in call.Arguments) + // A block only holds what CanIntroduceNamedArgument admitted. + Debug.Assert(!call.Method.IsAccessor || call.Method.AccessorOwner!.SymbolKind == SymbolKind.Indexer); + int nameableArgumentCount = NameableArgumentCount(call); + for (int i = 0; i < nameableArgumentCount; i++) { - if (arg.MatchLdLoc(v)) + if (call.Arguments[i].MatchLdLoc(v)) { - return FindResult.NamedArgument(arg, arg); + return FindResult.NamedArgument(call.Arguments[i], call.Arguments[i]); } } return FindResult.Stop; From 1d9b5d05124aab30993c445dcda2c5e7dc64adc7 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Mon, 24 Aug 2026 19:50:10 +0200 Subject: [PATCH 2/4] Keep an argument an override redeclares a default for An argument that repeats its parameter's default value may be left out, but the value was only ever compared against the method the call instruction names - for a virtual call the base declaration, since that is the slot the compiler emits. The shortened form binds against the receiver's static type, where an override is free to declare a different default, and the recompiled code then passes that one instead. Calls have had this since optional arguments were introduced; opening indexer accesses to omission brought it to element accesses too. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- .../Correctness/OverloadResolution.cs | 68 +++++++++++++++++++ .../TestCases/Pretty/NamedArguments.cs | 29 ++++++++ .../TestCases/Pretty/OptionalArguments.cs | 41 +++++++++++ ICSharpCode.Decompiler/CSharp/CallBuilder.cs | 45 +++++++++++- 4 files changed, 181 insertions(+), 2 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/OverloadResolution.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/OverloadResolution.cs index 412d61282..6de49b310 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/OverloadResolution.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/OverloadResolution.cs @@ -33,6 +33,10 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness ConstructorTest(); TestIndexer(); TestIndexerWithNamedArguments(); + TestRedeclaredDefaultValues(); +#if !MCS2 + TestNamedWithOmittedOptional(); +#endif Issue1281(); Issue1747(); CallAmbiguousOutParam(); @@ -331,6 +335,70 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness } #endregion + #region Named arguments with omitted optional arguments + // mcs 2.6.4 crashes while emitting a call that names its arguments and leaves an optional + // one out. +#if !MCS2 + static void TestNamedWithOmittedOptional() + { + var obj = new NamedOptionalTests(); + obj.M(b: Trace(2), a: Trace(1)); + obj.N(y: Trace(1), x: Trace(2)); + obj.N(z: Trace(1), x: Trace(2)); + } + + class NamedOptionalTests + { + public void M(int a, int b, int c = 3) + { + Console.WriteLine("M(" + a + ", " + b + ", " + c + ")"); + } + + public void N(int x, int y = 10, int z = 20) + { + Console.WriteLine("N(" + x + ", " + y + ", " + z + ")"); + } + } +#endif + #endregion + + #region Redeclared default values + static void TestRedeclaredDefaultValues() + { + var derived = new DerivedDefaultValue(); + Console.WriteLine(derived[1, 10]); + Console.WriteLine(derived.Method(1, 10)); + } + + class BaseDefaultValue + { + public virtual int this[int x, int y = 10] { + get { + return x + y; + } + } + + public virtual int Method(int x, int y = 10) + { + return x + y; + } + } + + class DerivedDefaultValue : BaseDefaultValue + { + public override int this[int x, int y = 20] { + get { + return x + y + 1; + } + } + + public override int Method(int x, int y = 20) + { + return x + y + 1; + } + } + #endregion + #region Indexer with named arguments static void TestIndexerWithNamedArguments() { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NamedArguments.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NamedArguments.cs index 4fed08cde..77d84517e 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NamedArguments.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NamedArguments.cs @@ -106,6 +106,22 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty } } + public int this[int a, int b, int c = 30] { + get { + return a; + } + set { + } + } + + public void UseOptional(int a, int b, int c = 3) + { + } + + public void UseTwoOptional(int x, int y = 10, int z = 20) + { + } + public void Use(int a, int b, int c) { } @@ -134,6 +150,19 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty this[y: Get(1), x: Get(2)] = 3; } + public void NamedArgsWithOmittedOptional() + { + UseOptional(b: Get(2), a: Get(1)); + UseTwoOptional(y: Get(1), x: Get(2)); + Use(this[b: Get(1), a: Get(2)], 0, 0); + this[b: Get(1), a: Get(2)] = 4; + } + + public void NamedArgsWithOmittedMiddleOptional() + { + UseTwoOptional(z: Get(1), x: Get(2)); + } + public void NamedArgsForIndexerNeedingCast() { Use(this[o: (object)((Get(1) == 1) ? "a" : "b"), i: Get(2)], 0, 0); diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArguments.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArguments.cs index 73ec58395..0be76eb59 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArguments.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/OptionalArguments.cs @@ -96,6 +96,38 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty } } + internal class BaseDefaultValue + { + public virtual int this[int x, int y = 10] { + get { + return x + y; + } + set { + } + } + + public virtual int Method(int x, int y = 10) + { + return x + y; + } + } + + internal class DerivedDefaultValue : BaseDefaultValue + { + public override int this[int x, int y = 20] { + get { + return x + y + 1; + } + set { + } + } + + public override int Method(int x, int y = 20) + { + return x + y + 1; + } + } + internal class BaseIndexer { public virtual int this[bool flag] { @@ -408,6 +440,15 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty } #endif + private void RedeclaredDefaultValues(DerivedDefaultValue derived) + { + // The calls go to the base declarations, whose defaults the override redeclares: + // leaving the argument out would pass the override's value instead. + Console.WriteLine(derived[1, 10]); + derived[1, 10] = 5; + Console.WriteLine(derived.Method(1, 10)); + } + private void Indexers(Indexer indexer, IndexerWithOverload overloaded) { Console.WriteLine(indexer[1]); diff --git a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs index c35e0e50a..dcf6f5e96 100644 --- a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs @@ -1237,6 +1237,37 @@ namespace ICSharpCode.Decompiler.CSharp } } + /// + /// Whether the arguments left out of the call are the default values of the member it + /// resolves to. They were compared against the parameters of the method the call + /// instruction names, which for a virtual call is the base declaration; an override may + /// redeclare a different default, and then leaving the argument out changes the value that + /// is passed. + /// + bool OmittedArgumentsAreDefaultsOf(ArgumentList argumentList, IMember? foundMember) + { + int argumentCount = argumentList.IsSetter ? argumentList.Length - 1 : argumentList.Length; + int omittedFrom = argumentList.GetActualArgumentCount(); + if (omittedFrom >= argumentCount) + return true; + if (foundMember is not IParameterizedMember foundParameterizedMember) + return false; + var parameters = foundParameterizedMember.Parameters; + // Names may leave out a parameter in the middle, so what was dropped is found through + // the map rather than by position. Its first entries are the target's. + var map = argumentList.ArgumentToParameterMap; + int firstParamIndex = map != null ? map.Count - argumentList.Length : 0; + for (int i = omittedFrom; i < argumentCount; i++) + { + int parameterIndex = map != null ? map[i + firstParamIndex] : i; + if (parameterIndex < 0 || parameterIndex >= parameters.Count) + return false; + if (!IsOptionalArgument(parameters[parameterIndex], argumentList.Arguments[i])) + return false; + } + return true; + } + bool IsOptionalArgument(IParameter parameter, TranslatedExpression arg) { if (!parameter.IsOptional) @@ -1354,8 +1385,17 @@ namespace ICSharpCode.Decompiler.CSharp OverloadResolutionErrors errors; while ((errors = IsUnambiguousCall(expectedTargetDetails, method, targetResolveResult, typeArguments, argumentList.GetArgumentResolveResults().ToArray(), argumentList.GetArgumentNames(), out foundMethod, - out var bestCandidateIsExpandedForm)) != OverloadResolutionErrors.None || bestCandidateIsExpandedForm != argumentList.IsExpandedForm) + out var bestCandidateIsExpandedForm)) != OverloadResolutionErrors.None + || bestCandidateIsExpandedForm != argumentList.IsExpandedForm + || !OmittedArgumentsAreDefaultsOf(argumentList, foundMethod)) { + if (errors == OverloadResolutionErrors.None && argumentList.FirstOptionalArgumentIndex >= 0) + { + // Resolution succeeded, so the omitted arguments are what is left: they do not + // match the defaults of the member found and have to be written out again. + argumentList.FirstOptionalArgumentIndex = -1; + continue; + } switch (errors) { case OverloadResolutionErrors.OutVarTypeMismatch: @@ -1884,7 +1924,8 @@ namespace ICSharpCode.Decompiler.CSharp IMember? foundMember; while (!IsUnambiguousAccess(expectedTargetDetails, targetResolveResult, method, - argumentList.GetArgumentResolveResults(), argumentList.GetArgumentNames(), out foundMember)) + argumentList.GetArgumentResolveResults(), argumentList.GetArgumentNames(), out foundMember) + || !OmittedArgumentsAreDefaultsOf(argumentList, foundMember)) { if (argumentList.FirstOptionalArgumentIndex >= 0) { From 2db2a92269f0cd4f60c1cc0489c35cbe6e37c813 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Tue, 25 Aug 2026 06:39:55 +0200 Subject: [PATCH 3/4] Resolve accesses and constructor calls through one ladder Three loops made an expression bind back to the member the IL names, and they were subsets of each other. The one for a property, indexer or event access could not tell why resolution had failed, so any failure spent the omitted arguments first, whatever the cause, and the steps that give up a name or an implicitly typed out variable had no equivalent at all. The one for a constructor tried the first two steps, cast the arguments once and gave up, with a comment about not looping forever. All three now run the same ladder. IsUnambiguousAccess answers with the same OverloadResolutionErrors an unresolvable call reports, so the step tried next follows from the error rather than from the order the loop happens to be written in, and a constructor passes no transformations because it names a type rather than a member. The arguments the ladder casts are passed as the array they live in with a count, which says at the call site both that the casts reach the argument list - the next attempt reads them back - and that a setter's assigned value is not among them. Decompiling 4038 types of the runtime and Newtonsoft.Json before and after gives identical output. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- ICSharpCode.Decompiler/CSharp/CallBuilder.cs | 209 ++++++++++--------- 1 file changed, 109 insertions(+), 100 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs index dcf6f5e96..dcbb43124 100644 --- a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs @@ -123,7 +123,7 @@ namespace ICSharpCode.Decompiler.CSharp } } - public IList GetArgumentResolveResultsDirect(int skipCount = 0) + public ResolveResult[] GetArgumentResolveResultsDirect(int skipCount = 0) { return Arguments .Skip(skipCount) @@ -1036,14 +1036,19 @@ namespace ICSharpCode.Decompiler.CSharp // >= 0 - the index of the first argument that can be removed, because it is optional // and is the default value of the parameter. int firstOptionalArgumentIndex = expressionBuilder.settings.OptionalArguments ? -2 : -1; - // Only an accessor written as an access takes its assigned value out of the argument - // list; one written as a call passes it like any other argument. The scan below and - // GetActualArgumentCount() have to agree on that, so it is decided once, here. + // Only an access takes the assigned value out of the argument list; an accessor + // written as a call passes it like any other argument. bool writtenAsMemberAccess = IsWrittenAsMemberAccess(method); + if (writtenAsMemberAccess && TakesAssignedValueLast(method) && argumentToParameterMap != null + && argumentToParameterMap[callArguments.Count - 1] != method.Parameters.Count - 1) + { + // An access writes the value from the last argument; in any other order there is + // no access syntax for it. + writtenAsMemberAccess = false; + } bool isSetter = writtenAsMemberAccess && TakesAssignedValueLast(method); - // A named argument of an indexer access names a parameter of the indexer, which the type - // system takes from the getter. The accessor being called may name the same parameters - // differently - C# cannot declare that, but other languages can. + // A name in an element access names a parameter of the indexer, which the type system + // takes from the getter; the accessor being called may name them differently. IReadOnlyList namedParameters = method.AccessorOwner is IProperty { IsIndexer: true } indexer ? indexer.Parameters : method.Parameters; @@ -1238,11 +1243,9 @@ namespace ICSharpCode.Decompiler.CSharp } /// - /// Whether the arguments left out of the call are the default values of the member it - /// resolves to. They were compared against the parameters of the method the call - /// instruction names, which for a virtual call is the base declaration; an override may - /// redeclare a different default, and then leaving the argument out changes the value that - /// is passed. + /// Whether the omitted arguments are the defaults of the member the shortened call resolves + /// to. They were compared against the method the call instruction names, which for a virtual + /// call is the base declaration, and an override may redeclare a different default. /// bool OmittedArgumentsAreDefaultsOf(ArgumentList argumentList, IMember? foundMember) { @@ -1299,13 +1302,46 @@ namespace ICSharpCode.Decompiler.CSharp private CallTransformation GetRequiredTransformationsForCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, ref TranslatedExpression target, ref ArgumentList argumentList, CallTransformation allowedTransforms, out IParameterizedMember? foundMethod) + { + var transform = GetRequiredTransformations(expectedTargetDetails, method, ref target, ref argumentList, + allowedTransforms, writtenAsMemberAccess: false, out var foundMember); + foundMethod = (IParameterizedMember?)foundMember; + return transform; + } + + /// + /// Finds the transformations an expression needs to bind back to the member the IL names, + /// cheapest first. With the expression is a + /// property, indexer or event access, which resolves against the accessor's owner. + /// + private CallTransformation GetRequiredTransformations(ExpectedTargetDetails expectedTargetDetails, IMethod method, + ref TranslatedExpression target, ref ArgumentList argumentList, CallTransformation allowedTransforms, + bool writtenAsMemberAccess, out IMember? foundMember) { CallTransformation transform = CallTransformation.None; + IMember boundMember = writtenAsMemberAccess ? method.AccessorOwner! : method; // initialize requireTarget flag bool requireTarget; ResolveResult? targetResolveResult; - if ((allowedTransforms & CallTransformation.RequireTarget) != 0) + if (writtenAsMemberAccess) + { + if (settings.AlwaysQualifyMemberReferences || boundMember.SymbolKind == SymbolKind.Indexer + || expressionBuilder.HidesVariableWithName(boundMember.Name)) + { + requireTarget = true; + } + else if (method.IsStatic) + { + requireTarget = !expressionBuilder.IsCurrentOrContainingType(method.DeclaringTypeDefinition); + } + else + { + requireTarget = target.Expression is not ThisReferenceExpression; + } + targetResolveResult = requireTarget ? target.ResolveResult : null; + } + else if ((allowedTransforms & CallTransformation.RequireTarget) != 0) { if (settings.AlwaysQualifyMemberReferences || expressionBuilder.HidesVariableWithName(method.Name)) { @@ -1379,20 +1415,35 @@ namespace ICSharpCode.Decompiler.CSharp } bool targetCasted = false; - bool argumentsCasted = false; + bool argumentsCasted = writtenAsMemberAccess && argumentList.GetActualArgumentCount() == 0; bool originalRequireTarget = requireTarget; - bool skipTargetCast = method.Accessibility <= Accessibility.Protected && expressionBuilder.IsBaseTypeOfCurrentType(method.DeclaringTypeDefinition); + bool skipTargetCast = !writtenAsMemberAccess + && method.Accessibility <= Accessibility.Protected && expressionBuilder.IsBaseTypeOfCurrentType(method.DeclaringTypeDefinition); OverloadResolutionErrors errors; - while ((errors = IsUnambiguousCall(expectedTargetDetails, method, targetResolveResult, typeArguments, - argumentList.GetArgumentResolveResults().ToArray(), argumentList.GetArgumentNames(), out foundMethod, - out var bestCandidateIsExpandedForm)) != OverloadResolutionErrors.None - || bestCandidateIsExpandedForm != argumentList.IsExpandedForm - || !OmittedArgumentsAreDefaultsOf(argumentList, foundMethod)) + while (true) { + bool expandedFormMismatch = false; + if (writtenAsMemberAccess) + { + errors = IsUnambiguousAccess(expectedTargetDetails, targetResolveResult, method, + argumentList.GetArgumentResolveResults(), argumentList.GetArgumentNames(), out foundMember); + } + else + { + errors = IsUnambiguousCall(expectedTargetDetails, method, targetResolveResult, typeArguments, + argumentList.GetArgumentResolveResults(), argumentList.GetArgumentNames(), + out var foundMethod, out bool bestCandidateIsExpandedForm); + foundMember = foundMethod; + expandedFormMismatch = bestCandidateIsExpandedForm != argumentList.IsExpandedForm; + } + if (errors == OverloadResolutionErrors.None && !expandedFormMismatch + && OmittedArgumentsAreDefaultsOf(argumentList, foundMember)) + { + break; + } if (errors == OverloadResolutionErrors.None && argumentList.FirstOptionalArgumentIndex >= 0) { - // Resolution succeeded, so the omitted arguments are what is left: they do not - // match the defaults of the member found and have to be written out again. + // The omitted arguments are not the defaults of the member found. argumentList.FirstOptionalArgumentIndex = -1; continue; } @@ -1443,7 +1494,8 @@ namespace ICSharpCode.Decompiler.CSharp } argumentsCasted = true; argumentList.UseImplicitlyTypedOut = false; - CastArguments(argumentList.Arguments, argumentList.ExpectedParameters); + CastArguments(argumentList.Arguments, argumentList.GetActualArgumentCount(), + argumentList.ExpectedParameters); } else if ((allowedTransforms & CallTransformation.RequireTarget) != 0 && !requireTarget) { @@ -1462,7 +1514,7 @@ namespace ICSharpCode.Decompiler.CSharp else { targetCasted = true; - target = target.ConvertTo(method.DeclaringType, expressionBuilder); + target = target.ConvertTo(boundMember.DeclaringType, expressionBuilder); targetResolveResult = target.ResolveResult; } } @@ -1483,7 +1535,7 @@ namespace ICSharpCode.Decompiler.CSharp continue; } // We've given up. - foundMethod = method; + foundMember = boundMember; break; } if ((allowedTransforms & CallTransformation.RequireTarget) != 0 && requireTarget) @@ -1600,9 +1652,13 @@ namespace ICSharpCode.Decompiler.CSharp return newObj; } - private void CastArguments(IList arguments, IList expectedParameters) + /// + /// Casts the first arguments in place - the retry loop reads them + /// back from the array on its next attempt. A setter's assigned value is past the count. + /// + private void CastArguments(TranslatedExpression[] arguments, int count, IParameter[] expectedParameters) { - for (int i = 0; i < arguments.Count; i++) + for (int i = 0; i < count; i++) { if (settings.AnonymousTypes && expectedParameters[i].Type.ContainsAnonymousType()) { @@ -1830,8 +1886,12 @@ namespace ICSharpCode.Decompiler.CSharp return OverloadResolutionErrors.None; } - bool IsUnambiguousAccess(ExpectedTargetDetails expectedTargetDetails, ResolveResult? target, IMethod method, - IList arguments, string[]? argumentNames, [NotNullWhen(true)] out IMember? foundMember) + /// + /// Resolves an access the way a call is resolved, so that the ladder can tell one that binds + /// to nothing from one that is merely missing an argument. + /// + OverloadResolutionErrors IsUnambiguousAccess(ExpectedTargetDetails expectedTargetDetails, ResolveResult? target, IMethod method, + ResolveResult[] arguments, string[]? argumentNames, out IMember? foundMember) { Log.WriteLine("IsUnambiguousAccess: Performing overload resolution for " + method); Log.WriteCollection(" Arguments: ", arguments); @@ -1843,7 +1903,7 @@ namespace ICSharpCode.Decompiler.CSharp EmptyList.Instance, isInvocationTarget: false) as MemberResolveResult; if (result == null || result.IsError) - return false; + return OverloadResolutionErrors.AmbiguousMatch; foundMember = result.Member; } else @@ -1852,15 +1912,15 @@ namespace ICSharpCode.Decompiler.CSharp if (method.AccessorOwner!.SymbolKind == SymbolKind.Indexer) { var or = new OverloadResolution(resolver.Compilation, - arguments.ToArray(), + arguments, argumentNames: argumentNames, typeArguments: Empty.Array, conversions: expressionBuilder.resolver.conversions); or.AddMethodLists(lookup.LookupIndexers(target)); if (or.BestCandidateErrors != OverloadResolutionErrors.None) - return false; + return or.BestCandidateErrors; if (or.IsAmbiguous) - return false; + return OverloadResolutionErrors.AmbiguousMatch; foundMember = or.GetBestCandidateWithSubstitutedTypeArguments(); } else @@ -1870,11 +1930,16 @@ namespace ICSharpCode.Decompiler.CSharp EmptyList.Instance, isInvocation: false) as MemberResolveResult; if (result == null || result.IsError) - return false; + return OverloadResolutionErrors.AmbiguousMatch; foundMember = result.Member; } } - return foundMember != null && IsAppropriateCallTarget(expectedTargetDetails, method.AccessorOwner, foundMember); + if (foundMember == null || !IsAppropriateCallTarget(expectedTargetDetails, method.AccessorOwner!, foundMember)) + { + foundMember = null; + return OverloadResolutionErrors.AmbiguousMatch; + } + return OverloadResolutionErrors.None; } /// @@ -1904,17 +1969,7 @@ namespace ICSharpCode.Decompiler.CSharp ExpressionWithResolveResult HandleAccessorCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, TranslatedExpression target, ArgumentList argumentList) { - bool requireTarget; - if (settings.AlwaysQualifyMemberReferences || method.AccessorOwner!.SymbolKind == SymbolKind.Indexer || expressionBuilder.HidesVariableWithName(method.AccessorOwner.Name)) - requireTarget = true; - else if (method.IsStatic) - requireTarget = !expressionBuilder.IsCurrentOrContainingType(method.DeclaringTypeDefinition); - else - requireTarget = !(target.Expression is ThisReferenceExpression); - bool targetCasted = false; bool isSetter = argumentList.IsSetter; - bool argumentsCasted = (isSetter && method.Parameters.Count == 1) || (!isSetter && method.Parameters.Count == 0); - var targetResolveResult = requireTarget ? target.ResolveResult : null; // Dropping every argument would turn an indexer access into a property access. if (argumentList.FirstOptionalArgumentIndex == 0 && method.AccessorOwner!.SymbolKind == SymbolKind.Indexer) @@ -1922,45 +1977,13 @@ namespace ICSharpCode.Decompiler.CSharp argumentList.FirstOptionalArgumentIndex = 1; } - IMember? foundMember; - while (!IsUnambiguousAccess(expectedTargetDetails, targetResolveResult, method, - argumentList.GetArgumentResolveResults(), argumentList.GetArgumentNames(), out foundMember) - || !OmittedArgumentsAreDefaultsOf(argumentList, foundMember)) - { - if (argumentList.FirstOptionalArgumentIndex >= 0) - { - // Unlike the casts below, writing the omitted arguments out again cannot change - // what the access means, so try that first. - argumentList.FirstOptionalArgumentIndex = -1; - } - else if (!argumentsCasted) - { - argumentsCasted = true; - CastArguments( - new ArraySegment(argumentList.Arguments, 0, argumentList.GetActualArgumentCount()), - argumentList.ExpectedParameters); - } - else if (!requireTarget) - { - requireTarget = true; - targetResolveResult = target.ResolveResult; - } - else if (!targetCasted) - { - targetCasted = true; - target = target.ConvertTo(method.AccessorOwner!.DeclaringType, expressionBuilder); - targetResolveResult = target.ResolveResult; - } - else - { - foundMember = method.AccessorOwner!; - break; - } - } + var transform = GetRequiredTransformations(expectedTargetDetails, method, ref target, ref argumentList, + CallTransformation.RequireTarget, writtenAsMemberAccess: true, out var foundMember); + Debug.Assert(foundMember != null); + bool requireTarget = (transform & CallTransformation.RequireTarget) != 0; var arguments = argumentList.GetArgumentExpressions().ToList(); - // The assigned value is not one of the arguments the ladder casts, so nothing it could - // try makes an access resolve that fails over the value's type. + // Not one of the arguments the ladder casts. TranslatedExpression value = isSetter ? argumentList.Arguments[argumentList.Length - 1] : default; var rr = new MemberResolveResult(target.ResolveResult, foundMember); @@ -2082,24 +2105,10 @@ namespace ICSharpCode.Decompiler.CSharp } else { - while (IsUnambiguousCall(expectedTargetDetails, method, null, Empty.Array, - argumentList.GetArgumentResolveResults().ToArray(), - argumentList.GetArgumentNames(), out _, - out var bestCandidateIsExpandedForm) != OverloadResolutionErrors.None || bestCandidateIsExpandedForm != argumentList.IsExpandedForm) - { - if (argumentList.AddNamesToPrimitiveValues) - { - argumentList.AddNamesToPrimitiveValues = false; - continue; - } - if (argumentList.FirstOptionalArgumentIndex >= 0) - { - argumentList.FirstOptionalArgumentIndex = -1; - continue; - } - CastArguments(argumentList.Arguments, argumentList.ExpectedParameters); - break; // make sure that we don't not end up in an infinite loop - } + // A constructor names its type, so neither qualification nor type arguments apply. + TranslatedExpression noTarget = default; + GetRequiredTransformations(expectedTargetDetails, method, ref noTarget, ref argumentList, + CallTransformation.None, writtenAsMemberAccess: false, out _); IType? returnTypeOverride = null; if (typeSystem.MainModule.TypeSystemOptions.HasFlag(TypeSystemOptions.NativeIntegersWithoutAttribute)) { From 28faea909958b209316948c1f3e6f1efc6697653 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Tue, 25 Aug 2026 06:40:17 +0200 Subject: [PATCH 4/4] Give readability names up on a copy The names that make a primitive value readable were written into the ArgumentNames array the call carries, so the step that gives them up again found them still there: for any call that already carried names of its own, turning them off was a no-op, and the ladder went on to cast instead. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- ICSharpCode.Decompiler/CSharp/CallBuilder.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs index dcbb43124..c50e49982 100644 --- a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs @@ -77,10 +77,11 @@ namespace ICSharpCode.Decompiler.CSharp && !ParameterNames.Any(string.IsNullOrEmpty)) { Debug.Assert(skipCount == 0); - if (argumentNames == null) - { - argumentNames = new string[Arguments.Length]; - } + // On a copy: giving these names up again must leave the ones that order the + // arguments untouched. + argumentNames = argumentNames == null + ? new string[Arguments.Length] + : (string[])argumentNames.Clone(); for (int i = 0; i < Arguments.Length; i++) {