Optional parameters with implicit default values

[ds5678]

Input code

.assembly extern System.Runtime
{
  .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A )                         // .?_....:
  .ver 4:0:0:0
}

.class public auto ansi abstract sealed beforefieldinit ICSharpCode.Decompiler.Tests.TestCases.ILPretty.Issue3469
    extends [System.Runtime]System.Object
{
    // Methods
    .method public hidebysig static 
        void M (
            [opt] int32 i,
            [opt] valuetype [System.Runtime]System.DateTime d
        ) cil managed 
    {
        .param [1] = int32(0)
        // Roslyn inserts, but Mono doesn't:
        // .param [2] = nullref
        // Method begins at RVA 0x2050
        // Code size 1 (0x1)
        .maxstack 8

        IL_0000: ret
    } // end of method Issue3469::M

} // end of class ICSharpCode.Decompiler.Tests.TestCases.ILPretty.Issue3469

Erroneous output

using System;
using System.Runtime.InteropServices;
namespace ICSharpCode.Decompiler.Tests.TestCases.ILPretty
{
	public static class Issue3469
	{
		public static void M(int i = 0, [Optional] DateTime d)
		{
		}
 	}
}

CS1737: Optional parameter cannot precede required parameters

Expected output

using System;
namespace ICSharpCode.Decompiler.Tests.TestCases.ILPretty
{
	public static class Issue3469
	{
		public static void M(int i = 0, DateTime d = default(DateTime))
		{
		}
 	}
}

Details

• Product in use: IL pretty unit test
• Version in use: latest commit (aff9649)
• Any other relevant information to the issue, or your interest in contributing a fix.
  • I am always willing to contribute fixes for my issues.
  • This assembly was compiled with the Unity Mono compiler.
  • If this is a step too far, I can automatically preprocess the assembly with AsmResolver before using ILSpy. I already do this in other situations, like removing public keys from InternalsVisibleToAttribute.

[ds5678]

If this is a step too far, I can automatically preprocess the assembly with AsmResolver before using ILSpy.

Serious. There'd be no hard feelings if you don't want it in the decompiler. A preprocessor is trivial:

foreach (MethodDefinition method in manager.GetAllMethods())
{
	foreach (Parameter parameter in method.Parameters)
	{
		ParameterDefinition? definition = parameter.Definition;
		if (definition is { IsOptional: true })
		{
			definition.HasDefault = true;
			definition.Constant ??= Constant.FromDefault(parameter.ParameterType);
		}
	}
}

[siegfriedpammer]

No, this is a bug and I fixed similar things in the past. Feel free to create a PR. Thank you very much!

[ds5678]

Which is preferred?

public static void M(int i = 0, DateTime d = default(DateTime))

public static void M([Optional, DefaultParameterValue(0)] int i, [Optional] DateTime d)

[greenozon]

Those belongs to diff C# versions

1st one -
Uses default parameter values introduced in C# 4.0 (2010).
default(DateTime) is allowed since C# 7.1

Aso, you could use type inference feature and write as
public static void M(int i = 0, DateTime d = default)

2nd one -
Uses attributes from System.Runtime.InteropServices (Optional and DefaultParameterValue).
These attributes are primarily used for COM interop and were available in C# 1.0 (part of .NET from the beginning).

[siegfriedpammer]

see also #2920

@greenozon this is not entirely correct... see https://sharplab.io/#v2:C4LglgNgPgAgTARgLACgYAYAEMEDoBKArgHbBgC2AprgJKmUBOA9gA4DKjAbmAMaUDOAblQwAzNjiYAwpgDeqTIuziYAFkwARSgDMAhoQjAACroa6qwRgAowpTLswBeTAFYAlHIVKAvl8VjsdS09A2NTc0pLBnxdAHcrAG0AeRYyJmJdCAAaTR19QxMzC0YANUzCSit3AF1bYHsPeRQlTF8UbyA=

if you compare the IL for the DefaultParameter and DefaultParameterRaw methods, you can see that int x = 5 is actually just syntactic sugar for [Optional, DefaultParameterValue(5)] int x.

So, looking at the original problem:

public static void M(int i = 0, [Optional] DateTime d) {}

leads to error CS1737: Optional parameters must appear after all required parameters

but this seems to work and is semantically equivalent:

public static void M([Optional, DefaultParameterValue(0)] int i, [Optional] DateTime d) {}

So I think, we need some extra validation in the type system that makes sure that all parameters after the first "natively supported" optional parameter are also "natively supported" optional parameters and if that's not the case just don't translate the Optional and DefaultParameterValue pseudo attributes when decoding the MetadataParameter instances.