VB -> C#: wrong conversion of optional struct or decimal ref parameter

[KunzeAndreas]

VB.Net input code

Private Shared Sub OptionalParams()
    FunctionWithOptionalParams()
End Sub

 Private Shared Sub FunctionWithOptionalParams(Optional ByRef structParam As TestStruct = Nothing, Optional ByRef decimalParam As Decimal = 0)
    structParam = New TestStruct
    decimalParam = 0
 End Sub

Friend Structure TestStruct
    Friend A As Boolean
End Structure

Erroneous output

 private static void OptionalParams()
 {
      TestStruct argstructParam = null;
      decimal argdecimalParam = 0m;
      FunctionWithOptionalParams(structParam: ref argstructParam, decimalParam: ref argdecimalParam);
 }

private static void FunctionWithOptionalParams([Optional, DefaultParameterValue(default(TestStruct))] ref TestStruct structParam, [Optional, DefaultParameterValue(0m)] ref decimal decimalParam)

Expected output

private static void OptionalParams()
{
    TestStruct argstructParam = default;
}

private static void FunctionWithOptionalParams([Optional] ref TestStruct structParam, [Optional, DefaultParameterValue(0)] ref decimal decimalParam)

Details

• Product in use: VS extension
• Version in use: 9.0.0.0
• Did you see it working in a previous version, which? no
• may be, there is a better way to express DefaultParameterValue for a structure

[Yozer]

Since in C# we have to provide the ref/out arguments anyway then maybe we could initialize them with correct defaults (I think we're already doing it with the exception of value parameters and default keyword as in this bug):

So this:

Private Shared Sub OptionalParams()
    FunctionWithOptionalParams()
End Sub

Private Shared Sub FunctionWithOptionalParams(
          Optional ByRef structParam As TestStruct = Nothing, 
          Optional ByRef decimalParam As Decimal = 1,
          Optional ByRef strParam As String = "test",
          Optional ByRef dateParam As Date = #2020-01-01#)
  structParam = New TestStruct
  decimalParam = 0
  strParam = "whatever"
  dateParam = #2021-01-01#
End Sub

could simply be this:

private static void OptionalParams()
{
    TestStruct structParam = default(TestStruct);
    decimal decimalParam = 1m;
    string strParam = "test";
    DateTime dateParam = new DateTime(637134336000000000L); // TODO, can we do better?
    FunctionWithOptionalParams(ref structParam, ref decimalParam, ref strParam, ref dateParam);
}

private static void FunctionWithOptionalParams(ref TestStruct structParam, ref decimal decimalParam, ref string strParam, ref DateTime dateParam)
{
    structParam = default(TestStruct);
    decimalParam = default(decimal);
    strParam = "whatever";
    dateParam = new DateTime(637450560000000000L);
}

I guess we still have to live with [Optional, DefaultParam] for ByValue parameters but ByRef could be simplified. @GrahamTheCoder what do you think?

[KunzeAndreas]

private static void FunctionWithOptionalParams(ref TestStruct structParam, ref decimal decimalParam, ref string strParam, ref DateTime dateParam)

would break VB code calling converted C# code (in VB you can omit optional ref params)

[GrahamTheCoder]

Thanks both, looks like there are a few things to fix here!

1. The pre-initialization of the ref parameter before calling should go from TestStruct argstructParam = null to TestStruct argstructParam = default
2. DefaultParameterValue(0m) -> DefaultParameterValue(0)
3. DefaultParameterValue(0.3m) -> ?
4. DefaultParameterValue(default(TestStruct)) -> ?

Point 1 and 2 should be very easy and could be done without further discussion.

I haven't looked in detail, but point 3 and/or 4 may be impossible while preserving the public interface for VB callers (as you mentioned). In your example, the method is private, so it'd be safe to remove the DefaultParameterValue attribute entirely really, it's only there for extra information. If we can't find a way, for cases we know it wont compile (e.g. structs) we could either remove the parameter, or leave the original VB of e.g. /* = 0.3 */ in place as a block comment.

[Yozer]

3. Could become [DecimalConstant(1, 0, 0u, 0u, 3u)]. DecimalConstant
4. I wasn't able to come up with something that would compile.

There is also an issue with converting methods that have Optional ByValue before Optional ByRef:

Private Shared Sub FunctionWithOptionalParams(
          Optional nonRef As String = "a",
          Optional ByRef strParam As String = "test")
End Sub

C# compiler will complain that optional params have to be at the end:

private static void FunctionWithOptionalParams(
   string nonRef = "a", 
   [Optional, DefaultParameterValue("test")] ref string strParam)
{
}

In this case, I didn't find a way to make it work - the easiest way to solve it was to refactor the code so it doesn't use optional byRefs.

[GrahamTheCoder]

Ah yes, nice one, I can see DecimalConstant in the decompilation of that VB, but there's no struct equivalent.

For the ordering issue, one solution would be to make nonRef non-optional in C# (i.e. still have the attribute, but not the default value) and pass the previous value at all call sites.

[KunzeAndreas]

Optional ByRef xxx As Short = 0S converts to [Optional, DefaultParameterValue(0)] out short xxx

should be [Optional, DefaultParameterValue((short)0)] out short xxx

[GrahamTheCoder]

Hopefully using AddExplicitConversion with isConst=true will deal with those sorts of issues: https://github.com/icsharpcode/CodeConverter/blob/master/CodeConverter/CSharp/TypeConversionAnalyzer.cs#L49