VB -> C#: Passing read-only property value by reference to a method

[joergenbech]

Summary

When passing a read-only property value to a method, the converted code will copy the property value to a temp variable (ok), pass the temp variable to the method (ok), and then after return assign the temp variable value to the property value (fail). The last step is not seen when decompiling the code that was compiled from the original VB.NET source.

I am in the process of migrating a large VB.NET code base to C#. A lot of old methods and functions use the ByRef keyword for parameters that should have been passed by value - presumably because the original developers thought this was the right thing to do for performance reasons.

The right thing to do is to remove all of the ByRef keywords that can be removed and this will be done eventually, but right now the problem is that the ICSharpCode code converter extension (and its online sibling) generates C# code that cannot be compiled because of this issue.

• ICSharpCode extension: Uses temp variable. Attempts to write value back after the call.
• ICSharpCode online: Uses temp variable. Attempts to write value back after the call.
• Telerik online: Does not use temp variable. Does not attempt to write value back after the call.
• Decompiler: Uses temp variable. Does not attempt to write value back after the call.

I would like to see the ICSharpCode extension generate the same type of code as seen in the decompiler. Or at least a quick explanation why it behaves like it does. One might argue that the extension does the right thing by highlighting a problem, but the argument against that is that it is probably more correct to generate code that is as close to what the decompiled code looks like.

Details

VB.Net input code

Module Module1

    Public Class TestClass
        Public **ReadOnly** Property Foo As String

		Public Sub New()
            Foo = "abc"
        End Sub
    End Class

    Sub Main()
        Test02()
    End Sub

    Private Sub Test02()
        Dim t As New TestClass
		**Test02Sub(t.Foo)**
	End Sub

    Private Sub Test02Sub(**ByRef** value As String)
        Console.WriteLine(value)
    End Sub

End Module

Erroneous output

(Using Code Converter extension v8.4.1.0 as well as https://codeconverter.icsharpcode.net/)

namespace VBApp01
{
	static class Module1
	{
		public class TestClass
		{
			public string Foo { get; private set; }

			public TestClass()
			{
				Foo = "abc";
			}
		}

		public static void Main()
		{
			Test02();
		}

		private static void Test02()
		{
			var t = new TestClass();
			string argvalue = t.Foo;
			Test02Sub(ref argvalue);
			**t.Foo = argvalue;**
		}

		private static void Test02Sub(ref string value)
		{
			Console.WriteLine(value);
		}
	}
}

Expected output

namespace VBApp01
{
	static class Module1
	{
		public class TestClass
		{
			public string Foo { get; private set; }

			public TestClass()
			{
				Foo = "abc";
			}
		}

		public static void Main()
		{
			Test02();
		}

		private static void Test02()
		{
			var t = new TestClass();
			string argvalue = t.Foo;
			Test02Sub(ref argvalue);
		}

		private static void Test02Sub(ref string value)
		{
			Console.WriteLine(value);
		}
	}
}

Details

Using:

• Visual Studio 2022 v17.1.0.
• Code Converter extension v8.4.1.0.
• Online ICSharpCode converter: https://codeconverter.icsharpcode.net/
• Online Telerik converter: https://converter.telerik.com/
• dnSpy decompiler

Output from Code Converter extension is seen above.

Output from Telerik online code converter (supposedly based on ICSharpCode). No attempt to write the value back, but also does not use a temporary variable:

private static void Test02()
    {
        TestClass t = new TestClass();
        Test02Sub(ref t.Foo);
    }

Output from dnSpy decompiler, when looking at the assembly compiled from the original code. Uses a temp variable like the ICSharpCode converter, but does not attempt to write the value back. This is the kind of code I expected from the extension:

private static void Test02()
{
	Module1.TestClass t = new Module1.TestClass();
	string foo = t.Foo;
	Module1.Test02Sub(ref foo);
}

[GrahamTheCoder]

Thanks for the comprehensive bug report. The Telerik version is indeed an old one - before the general of passing properties by ref was tackled at all.
It sounds like this should be a very simple fix to just skip the reassignment in the case of a readonly property.
I'll try to get to it sometime this week, but I'm also happy to accept a PR (just mention here if you are working on it).

[GrahamTheCoder]

The fix will probably need to go somewhere around here: https://github.com/icsharpcode/CodeConverter/pull/566/files#r817528266

[joergenbech]

No, I am not familiar with the code base, so I won't be working on this. Just wanted to let you know about the discrepancy.

[GrahamTheCoder]

Fix released in 8.4.6. If you try it, let me know whether it's now working as expected, thanks!

[joergenbech]

Thank you. I compared the output from 8.4.1 (which was the one I happened to have installed) with that of 8.4.6, and the CS0200 problem appears to have been fixed.