diff --git a/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs b/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs index a00fc4425..37fea77c9 100644 --- a/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs @@ -295,6 +295,12 @@ namespace ICSharpCode.Decompiler.Tests await RunCS(options: options); } + [Test] + public async Task LambdaOptionalAndParamsParameters([ValueSource(nameof(roslynOnlyOptions))] CompilerOptions options) + { + await RunCS(options: options); + } + [Test] public async Task NullPropagation([ValueSource(nameof(roslynOnlyOptions))] CompilerOptions options) { diff --git a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs index b9bf32277..faa5a7456 100644 --- a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs @@ -824,6 +824,12 @@ namespace ICSharpCode.Decompiler.Tests await RunForLibrary(cscOptions: cscOptions); } + [Test] + public async Task LambdaOptionalAndParamsParameters([ValueSource(nameof(roslyn4OrNewerOptions))] CompilerOptions cscOptions) + { + await RunForLibrary(cscOptions: cscOptions); + } + [Test] public async Task RefStructInterfaces([ValueSource(nameof(roslyn4OrNewerOptions))] CompilerOptions cscOptions) { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/LambdaOptionalAndParamsParameters.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/LambdaOptionalAndParamsParameters.cs new file mode 100644 index 000000000..cde103f48 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/LambdaOptionalAndParamsParameters.cs @@ -0,0 +1,68 @@ +#pragma warning disable CS9099, CS9100 +using System; +using System.Reflection; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness +{ + class LambdaOptionalAndParamsParameters + { + static void Main() + { +#if CS120 && !NET40 + ReportMetadata(); + ReportCalls(); +#endif + } + +#if CS120 && !NET40 + delegate int OptionalFunc(int x = 5); + + delegate int PlainFunc(int x); + + delegate void ParamsAction(params int[] xs); + + delegate void PlainAction(int[] xs); + + // A lambda's parameter list does not have to repeat what the delegate declares: it may + // state a different default, one the delegate does not have, or none where the delegate + // has one, and the same for the params modifier. Metadata records what the lambda itself + // declared, and reflection reports that rather than the delegate's, so the decompiled + // lambda has to carry the lambda's own list back. + static void Report(string name, Delegate d) + { + ParameterInfo p = d.Method.GetParameters()[0]; + Console.WriteLine("{0}: hasDefault={1} default={2} params={3}", + name, + p.HasDefaultValue, + p.HasDefaultValue ? p.DefaultValue : "none", + p.IsDefined(typeof(ParamArrayAttribute), inherit: false)); + } + + static void ReportMetadata() + { + Report("DefaultOnlyInDelegate", (OptionalFunc)((int x) => x * 2)); + Report("DefaultOnlyInLambda", (PlainFunc)((int x = 3) => x * 2)); + Report("DefaultDiffersFromDelegate", (OptionalFunc)((int x = 7) => x * 2)); + Report("DefaultAgreesWithDelegate", (OptionalFunc)((int x = 5) => x * 2)); + Report("ParamsOnlyInDelegate", (ParamsAction)((int[] xs) => Console.WriteLine(xs.Length))); + Report("ParamsOnlyInLambda", (PlainAction)((params int[] xs) => Console.WriteLine(xs.Length))); + } + + // The value a caller gets for an omitted argument comes from the delegate, never from the + // lambda, so these stay the same whatever the lambda declared. + static void ReportCalls() + { + OptionalFunc noDefault = (int x) => x * 2; + OptionalFunc otherDefault = (int x = 7) => x * 2; + Console.WriteLine(noDefault()); + Console.WriteLine(otherDefault()); + Console.WriteLine(otherDefault(1)); + + ParamsAction expandedByDelegate = (int[] xs) => Console.WriteLine(xs.Length); + expandedByDelegate(1, 2, 3); + PlainAction notExpanded = (params int[] xs) => Console.WriteLine(xs.Length); + notExpanded(new int[2]); + } +#endif + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/LambdaOptionalAndParamsParameters.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/LambdaOptionalAndParamsParameters.cs new file mode 100644 index 000000000..ba8f6246f --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/LambdaOptionalAndParamsParameters.cs @@ -0,0 +1,99 @@ +#pragma warning disable CS9099, CS9100 +using System; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty +{ + internal class LambdaOptionalAndParamsParameters + { + public delegate void ParamsAction(params int[] xs); + + public delegate int OptionalFunc(int x = 5); + + public delegate int PlainFunc(int x); + + // A lambda states 'params' and defaults on its own account: what the delegate declares + // is the delegate's, and reflection over the lambda's method reports only the lambda's. + // Roslyn records the modifier on the anonymous function's method from version 5 on, so + // the cases that need it back are guarded on ROSLYN5. + + private int total; + +#if ROSLYN5 + public ParamsAction ParamsStatementBody() + { + return (params int[] xs) => { + total += xs.Length; + Console.WriteLine(xs.Length); + }; + } +#endif + +#if ROSLYN5 + public ParamsAction ParamsSingleStatementBody() + { + return (params int[] xs) => { + Console.WriteLine(xs.Length); + }; + } +#endif + + public OptionalFunc OptionalStatementBody() + { + return (int x = 5) => { + total += x; + return x * 2; + }; + } + + public OptionalFunc OptionalExpressionBody() + { + return (int x = 5) => x * 2; + } + + public ParamsAction ParamsWithoutParameterList() + { + return delegate { + }; + } + + public OptionalFunc OptionalWithoutParameterList() + { + return delegate { + return 1; + }; + } + + // The lambda's own defaults and params modifier can differ from the delegate's; the metadata + // records the lambda's, so that is what must round-trip. + public OptionalFunc OptionalDifferentDefault() + { + return (int x = 7) => x * 2; + } + + public OptionalFunc OptionalOnlyInDelegate() + { + return (int x) => x * 2; + } + + public PlainFunc OptionalOnlyInLambda() + { + return (int x = 3) => x * 2; + } + + public ParamsAction ParamsOnlyInDelegate() + { + return (int[] xs) => { + Console.WriteLine(xs.Length); + }; + } + +#if ROSLYN5 + public Action ParamsOnlyInLambda() + { + return (params int[] xs) => { + Console.WriteLine(xs.Length); + }; + } +#endif + } +} diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 9d7a329fe..031bc591e 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -2627,7 +2627,6 @@ namespace ICSharpCode.Decompiler.CSharp let v = ident.GetILVariable() where v != null && v.Function == function && v.Kind == VariableKind.Parameter select ident).Any(); - bool isLambda = false; if (ame.Parameters.Any(p => p.Type is null)) { @@ -2643,13 +2642,41 @@ namespace ICSharpCode.Decompiler.CSharp && (parametersAreUsed || (ParameterTypesAreAccessible(function) && ParametersAreNameable(function)))) { // Lambdas cover statement bodies too. Anonymous method syntax remains where - // dropping the parameter list is the better rendering: for ref/out/in and - // params parameters (expressible in an explicitly typed lambda list, but - // conservatively left alone), and for unused parameters that a list would have - // to name or type from nothing to keep. The parameter-list-less "delegate {}" - // form is compatible with any delegate signature, so it is always legal there. + // dropping the parameter list is the better rendering: for ref/out/in + // parameters, for params parameters when the C# 12 lambda form is not + // available (the modifier is stripped below), and for unused parameters that + // a list would have to name or type from nothing to keep. The + // parameter-list-less "delegate {}" form is compatible with any delegate + // signature, so it is always legal there. isLambda = true; } + // 'params' and parameter default values are only legal on the explicitly typed + // parameter list of a lambda, and only since C# 12; and a list that is about to be + // dropped cannot carry them at all. Everywhere else they are decorative - the + // delegate type still declares both, and that is what call sites bind against. + if (settings.LambdaOptionalAndParamsParameters + && (isLambda || parametersAreUsed) + && ame.Parameters.All(p => p.Type is not null)) + { + // Only what the anonymous function's own metadata declares is written. A lambda + // may state a different default than its target delegate, or none where the + // delegate has one, and reflection over the lambda's method reports what the + // lambda declared - so taking either from the delegate's Invoke would change + // what the recompiled assembly says. The delegate type keeps declaring both, + // and call sites bind against it, so nothing is lost by leaving them out here. + + // An anonymous method cannot declare either, in any language version. + if (ame.Parameters.Any(p => p.IsParams || p.DefaultExpression is not null)) + isLambda = true; + } + else + { + foreach (var p in ame.Parameters) + { + p.IsParams = false; + p.DefaultExpression?.Detach(); + } + } // Remove the parameter list from an AnonymousMethodExpression if the parameters are not used in the method body if (!isLambda && !parametersAreUsed) { diff --git a/ICSharpCode.Decompiler/DecompilerSettings.cs b/ICSharpCode.Decompiler/DecompilerSettings.cs index 3a5f0537b..4458a763f 100644 --- a/ICSharpCode.Decompiler/DecompilerSettings.cs +++ b/ICSharpCode.Decompiler/DecompilerSettings.cs @@ -848,6 +848,14 @@ namespace ICSharpCode.Decompiler [DecompilerSetting(CSharp.LanguageVersion.CSharp12_0)] public partial bool InlineArrays { get; set; } + /// + /// Gets/sets whether lambda parameter lists may declare 'params' and parameter default + /// values. When disabled, these modifiers are dropped from anonymous functions instead. + /// + [Description("DecompilerSettings.LambdaOptionalAndParamsParameters")] + [DecompilerSetting(CSharp.LanguageVersion.CSharp12_0)] + public partial bool LambdaOptionalAndParamsParameters { get; set; } + /// /// Gets/Sets whether C# 14.0 extension members should be transformed. /// diff --git a/ILSpy/Properties/Resources.Designer.cs b/ILSpy/Properties/Resources.Designer.cs index 28a6ade9a..fe9a420ec 100644 --- a/ILSpy/Properties/Resources.Designer.cs +++ b/ILSpy/Properties/Resources.Designer.cs @@ -1361,6 +1361,15 @@ namespace ICSharpCode.ILSpy.Properties { } } + /// + /// Looks up a localized string similar to 'params' and optional parameters in lambdas. + /// + public static string DecompilerSettings_LambdaOptionalAndParamsParameters { + get { + return ResourceManager.GetString("DecompilerSettings.LambdaOptionalAndParamsParameters", resourceCulture); + } + } + /// /// Looks up a localized string similar to Use nint/nuint types. /// diff --git a/ILSpy/Properties/Resources.resx b/ILSpy/Properties/Resources.resx index 73d84bc0d..d83ec092b 100644 --- a/ILSpy/Properties/Resources.resx +++ b/ILSpy/Properties/Resources.resx @@ -486,6 +486,9 @@ Are you sure you want to continue? IsUnmanagedAttribute on type parameters should be replaced with 'unmanaged' constraints + + 'params' and optional parameters in lambdas + Use nint/nuint types