Don't write back into a method call when it's ByRef argument

[TymurGubayev]

Problem

ByRef argument w/o parentheses is treated like a writable variable, even if it's a method:

Sub S(ByRef i as Object)
    M(GetI)
End Sub
Function GetI() As Integer : End Function

becomes

public bool M(ref object i)
{
    object argi1 = GetI();
    M(ref argi1);
    GetI() = argi1; //compiler error
}

public int GetI()
{
    return default;
}

Solution

• Any comments on the approach taken, its consistency with surrounding code, etc.
  The adjustment itself is very minor: return RefConversion.PreAssigment from GetRefConversion when the symbol is an IMethodSymbol.

• Which part of this PR is most in need of attention/improvement?
  I've excluded ReturnsByRef == false && ReturnsByRefReadonly == false, same as with properties. This generates code like this atm:

  object argi2 = byrefThis.RefProp;
  M(ref argi2);
  byrefThis.RefProp = Conversions.ToInteger(argi2);
  
  object argi3 = byrefThis.GetRefVal();
  M(ref argi3);
  byrefThis.GetRefVal() = Conversions.ToInteger(argi3);

  I haven't added any tests for this though, because of the whole problematic with solution tests involving C# ref returns from VB.NET.

  Also, this code isn't necessarily correct if the property or method involved has side effects. It should use a temporary ref variable, something like this:

  ref int tmp = ref byrefThis.GetRefVal();
  object argi3 = tmp;
  M(ref argi3);
  tmp = (int)(argi3);

  Unless there is an easy way to do that, I'd say it's out of scope of this PR.

• At least one test covering the code changed

[GrahamTheCoder]

I think there is some existing code that uses ref locals. I'll try to find it when I have a minute.

[GrahamTheCoder]

This is the code I was thinking of in case it helps:

CodeConverter/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs

Lines 878 to 881 in 9e929e1

	if (_semanticModel.GetOperation(vbExpr.SkipIntoParens()).IsAssignableExpression()) {
	forceType = SyntaxFactory.RefType(ValidSyntaxFactory.VarType);
	expr = SyntaxFactory.RefExpression(expr);
	}

Test example output:

CodeConverter/Tests/CSharp/StatementTests/MethodStatementTests.cs

Line 452 in 9e929e1

ref var withBlock = ref myArray[0];

Just a random thought on the issues of testing: I wonder whether we can get around the error by referencing a compiled binary rather than source code which contains a ref return. Either a ref return in the base class library or one we compile manually.

I'll merge this as-is since it seems like an improvement over what's there now.

[TymurGubayev]

Just a random thought on the issues of testing: I wonder whether we can get around the error by referencing a compiled binary rather than source code which contains a ref return. Either a ref return in the base class library or one we compile manually.

I checked what's present in the CharacterizationTestSolution, and it's System.Span-related things from netstandard.dll.
I'd prefer custom binary though, because it'd give us more control, and we could maybe use it for Interop-related tests as well.