Fix for #1043 (#1044)

Field property getter returns non-value types by reference instead of by copy.
Fixes #1043

* Minor code clarity cleanup in GenerateFieldSetter. No behavior changed.
* Fix incorrect code generated in some cases.
* Test for fields getters returning references.

Author: rokups

src/Generator/Generators/CSharp/CSharpSources.cs

@@ -906,13 +906,11 @@ private void GenerateFieldSetter(Field field, Class @class, QualifiedType fieldT
             else
             {
                 var name = @class.Layout.Fields.First(f => f.FieldPtr == field.OriginalPtr).Name;
-                var printed = TypePrinter.PrintNative(@class);
-                ctx.ReturnVarName = string.Format("{0}{1}{2}",
-                    @class.IsValueType
-                        ? Helpers.InstanceField
-                        : $"(({printed}*) {Helpers.InstanceIdentifier})",
-                    @class.IsValueType ? "." : "->",
-                    SafeIdentifier(name));
+                var identifier = SafeIdentifier(name);
+                if (@class.IsValueType)
+                    ctx.ReturnVarName = $"{Helpers.InstanceField}.{identifier}";
+                else
+                    ctx.ReturnVarName = $"(({TypePrinter.PrintNative(@class)}*){Helpers.InstanceIdentifier})->{identifier}";
             }
             param.Visit(marshal);
 
@@ -1174,13 +1172,24 @@ private static Property GetActualProperty(Property property, Class c)
         private void GenerateFieldGetter(Field field, Class @class, QualifiedType returnType)
         {
             var name = @class.Layout.Fields.First(f => f.FieldPtr == field.OriginalPtr).Name;
+            String returnVar;
+            if (@class.IsValueType)
+                returnVar = $@"{Helpers.InstanceField}.{SafeIdentifier(name)}";
+            else
+            {
+                returnVar = $@"(({TypePrinter.PrintNative(@class)}*) {Helpers.InstanceIdentifier})->{SafeIdentifier(name)}";
+                // Class field getter should return a reference object instead of a copy. Wrapping `returnVar` in
+                // IntPtr ensures that non-copying object constructor is invoked.
+                Class typeClass;
+                if (field.Type.TryGetClass(out typeClass) && !typeClass.IsValueType)
+                    returnVar = $"new {CSharpTypePrinter.IntPtrType}(&{returnVar})";
+            }
+
             var ctx = new CSharpMarshalContext(Context)
             {
                 ArgName = field.Name,
                 Declaration = field,
-                ReturnVarName = $@"{(@class.IsValueType ? Helpers.InstanceField :
-                    $"(({TypePrinter.PrintNative(@class)}*) {Helpers.InstanceIdentifier})")}{
-                    (@class.IsValueType ? "." : "->")}{SafeIdentifier(name)}",
+                ReturnVarName = returnVar,
                 ReturnType = returnType
             };
             ctx.PushMarshalKind(MarshalKind.NativeField);

src/Generator/Generators/CSharp/CSharpSourcesExtensions.cs

@@ -132,8 +132,14 @@ private static void ThrowException(CSharpSources gen, Class @class)
                     s.Arguments.Select(typePrinter.VisitTemplateArgument))}>"));
             var typeArguments = string.Join(", ", @class.TemplateParameters.Select(p => p.Name));
             var managedTypes = string.Join(", ", @class.TemplateParameters.Select(p => $"typeof({p.Name}).FullName"));
+
+            if (managedTypes.Length > 0)
+                managedTypes = $@"string.Join("", "", new[] {{ {managedTypes} }})";
+            else
+                managedTypes = "\"\"";
+
             gen.WriteLine($"throw new ArgumentOutOfRangeException(\"{typeArguments}\", "
-                + $@"string.Join("", "", new[] {{ {managedTypes} }}), "
+                + $@"{managedTypes}, "
                 + $"\"{@class.Visit(typePrinter)} maps a C++ template class and therefore it only supports a limited set of types and their subclasses: {supportedTypes}.\");");
         }
     }

tests/Common/Common.Tests.cs

@@ -525,6 +525,15 @@ public unsafe void TestSingleArgumentCtorToCastOperator()
         Assert.AreEqual(classB.Value, classC.Value);
     }
 
+    [Test]
+    public unsafe void TestFieldRef()
+    {
+        var classD = new ClassD(10);
+        var fieldRef = classD.Field;
+        fieldRef.Value = 20;
+        Assert.AreEqual(20, classD.Field.Value);
+    }
+
     [Test]
     public unsafe void TestDecltype()
     {

tests/Common/Common.cpp

@@ -469,6 +469,11 @@ ClassC::ClassC(const ClassB &x)
     Value = x.Value;
 }
 
+ClassD::ClassD(int value)
+    : Field(value)
+{
+}
+
 void DelegateNamespace::Nested::f1(void (*)())
 {
 }

tests/Common/Common.h

@@ -771,6 +771,14 @@ class DLL_API ClassC
     int Value;
 };
 
+class DLL_API ClassD
+{
+public:
+    ClassD(int value);
+    // Accessing this field should return reference, not a copy.
+    ClassA Field;
+};
+
 // Test decltype
 int Expr = 0;
 DLL_API decltype(Expr) TestDecltype()