From bd7f8f9e0901c4f59aafe8933e68220a36de41d4 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Tue, 25 Aug 2026 06:38:22 +0200 Subject: [PATCH] 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;