Fixed issues with methods which return via buffer.

* Fixed return by buffer for record types on instance methods on Windows x64.
* Fixed incorrect void return type on functions with return by buffer semantics. (It should've been the return buffer pointer type again.)
* Changed return buffer type on virtual function pointers from out byref to pointer to make it easier to properly implement virtual methods.

Fixes #142

Biohazrd.CSharp/CSharpLibraryGenerator.Functions.cs

@@ -93,9 +93,9 @@ private void EmitFunctionDllImport(VisitorContext context, EmitFunctionContext e
             AccessModifier accessibility = declaration.IsInstanceMethod ? AccessModifier.Private : declaration.Accessibility;
             Writer.Write($"{accessibility.ToCSharpKeyword()} static extern ");
 
-            // If the return value is passed by reference, the return type is void
+            // If the return value is passed by reference, the return type is the return buffer pointer
             if (declaration.ReturnByReference)
-            { Writer.Write("void"); }
+            { WriteTypeAsReference(context, declaration, declaration.ReturnType); }
             else
             { WriteType(context, declaration, declaration.ReturnType); }
 
@@ -282,7 +282,10 @@ private void EmitFunctionParameterList(VisitorContext context, EmitFunctionConte
                     if (!first)
                     { Writer.Write(", "); }
 
-                    Writer.Write("out ");
+                    if (!declaration.IsVirtual)
+                    { Writer.Write("out "); }
+                    else if (mode == EmitParameterListMode.TrampolineArguments)
+                    { Writer.Write("&"); }
 
                     if (writeTypes)
                     {

Biohazrd.CSharp/CSharpLibraryGenerator.WriteType.cs

@@ -126,21 +126,10 @@ private string GetTypeAsString(VisitorContext context, TranslatedDeclaration dec
 
                         string functionPointerResult = $"delegate* {callingConventionString}<";
 
-                        // This is a workaround for the fact that VTable function pointers have been modified to add the implicit this and retbuf parameters
-                        // The retbuf parameter is expected to be translated as a C# out parameter, but we have no way to express this in FunctionPointerTypeReference.
-                        bool __HACK__emitSecondParameterAsOut = declaration is TranslatedVTableEntry { MethodDeclaration: not null } vTableEntry
-                            // https://github.com/dotnet/roslyn/issues/47676
-                            && vTableEntry.MethodDeclaration!.ReturnType.MustBePassedByReference();
-
-                        int i = 0;
                         foreach (TypeReference parameterType in functionPointer.ParameterTypes)
                         {
-                            if (__HACK__emitSecondParameterAsOut && i == 1)
-                            { functionPointerResult += "out "; }
-
                             functionPointerResult += GetTypeAsString(context, declaration, parameterType);
                             functionPointerResult += ", ";
-                            i++;
                         }
 
                         functionPointerResult += GetTypeAsString(context, declaration, functionPointer.ReturnType);

Biohazrd/#Declarations/TranslatedFunction.cs

@@ -29,7 +29,6 @@ internal TranslatedFunction(TranslatedFile file, FunctionDecl function)
         {
             MangledName = function.Handle.Mangling.ToString();
             ReturnType = new ClangTypeReference(function.ReturnType);
-            ReturnByReference = function.ReturnType.MustBePassedByReference();
 
             // Enumerate parameters
             ImmutableArray<TranslatedParameter>.Builder parametersBuilder = ImmutableArray.CreateBuilder<TranslatedParameter>(function.Parameters.Count);
@@ -64,6 +63,9 @@ internal TranslatedFunction(TranslatedFile file, FunctionDecl function)
                 IsConst = false;
             }
 
+            // Determine if return value must be passed by reference
+            ReturnByReference = function.ReturnType.MustBePassedByReference(isForInstanceMethodReturnValue: IsInstanceMethod);
+
             // Handle operator overloads
             ref PathogenOperatorOverloadInfo operatorOverloadInfo = ref function.GetOperatorOverloadInfo();
 

Biohazrd/#Declarations/TranslatedParameter.cs

@@ -14,7 +14,7 @@ internal unsafe TranslatedParameter(TranslatedFile file, ParmVarDecl parameter)
             : base(file, parameter)
         {
             Type = new ClangTypeReference(parameter.Type);
-            ImplicitlyPassedByReference = parameter.Type.MustBePassedByReference();
+            ImplicitlyPassedByReference = parameter.Type.MustBePassedByReference(isForInstanceMethodReturnValue: false);
 
             DefaultValue = parameter.TryComputeConstantValue(out TranslationDiagnostic? diagnostic);
             Diagnostics = Diagnostics.AddIfNotNull(diagnostic);

Biohazrd/#Declarations/TranslatedRecord.cs

@@ -28,7 +28,7 @@ internal unsafe TranslatedRecord(TranslationUnitParser parsingContext, Translate
             if (!record.Handle.IsDefinition)
             { throw new ArgumentException("Only defining records can be translated!"); }
 
-            MustBePassedByReference = record.MustBePassedByReference();
+            MustBePassedByReference = record.MustBePassedByReference(isForInstanceMethodReturnValue: false);
 
             if (record.IsStruct)
             { Kind = RecordKind.Struct; }

Biohazrd/#Declarations/TranslatedVTableEntry.cs

@@ -40,7 +40,7 @@ internal TranslatedVTableEntry(TranslationUnitParser parsingContext, TranslatedF
                     // Ideally we should be able to encode this some other way.
                     for (int i = 0; i < functionTypeReference.ParameterTypes.Length; i++)
                     {
-                        if (functionTypeReference.ParameterTypes[i] is ClangTypeReference clangType && clangType.ClangType.MustBePassedByReference())
+                        if (functionTypeReference.ParameterTypes[i] is ClangTypeReference clangType && clangType.ClangType.MustBePassedByReference(isForInstanceMethodReturnValue: false))
                         {
                             functionTypeReference = functionTypeReference with
                             {
@@ -51,12 +51,14 @@ internal TranslatedVTableEntry(TranslationUnitParser parsingContext, TranslatedF
 
                     //TODO: This depends on the calling convention
                     // Add the retbuf parameter if necessary
-                    if (functionType.ReturnType.MustBePassedByReference())
+                    if (functionType.ReturnType.MustBePassedByReference(isForInstanceMethodReturnValue: true))
                     {
+                        PointerTypeReference returnBufferType = new(functionTypeReference.ReturnType);
+
                         functionTypeReference = functionTypeReference with
                         {
-                            ParameterTypes = functionTypeReference.ParameterTypes.Insert(0, functionTypeReference.ReturnType),
-                            ReturnType = VoidTypeReference.Instance
+                            ParameterTypes = functionTypeReference.ParameterTypes.Insert(0, returnBufferType),
+                            ReturnType = returnBufferType
                         };
                     }
 

Biohazrd/ClangSharpExtensions.cs

@@ -58,13 +58,18 @@ public static AccessModifier ToTranslationAccessModifier(this CX_CXXAccessSpecif
                 _ => AccessModifier.Public // The access specifier is invalid for declarations which aren't members of a record, so they are translated as public.
             };
 
-        internal static bool RecordMustBePassedByReference(this CXCursor cursor)
+        internal static bool RecordMustBePassedByReference(this CXCursor cursor, bool isForInstanceMethodReturnValue)
         {
             if (!cursor.IsDeclaration || cursor.DeclKind < CX_DeclKind.CX_DeclKind_FirstRecord || cursor.DeclKind > CX_DeclKind.CX_DeclKind_LastRecord)
             { throw new ArgumentException("The cursor must be a record declaration.", nameof(cursor)); }
 
             // Note: These rules assume Microsoft x64 ABI, need to evaluate for x86 and non-Windows x64
 
+            // On x64 Windows, user-defined records are _always_ returned by reference for instance methods
+            // See https://github.com/InfectedLibraries/Biohazrd/issues/142
+            if (isForInstanceMethodReturnValue)
+            { return true; }
+
             // ArgPassingRestrictions only covers cases like having a copy constructor or something similar
             // It (surpririsingly) doesn't handle cases involving the size of the record
             if (cursor.GetRecordArgPassingRestrictions() != PathogenArgPassingKind.CanPassInRegisters)
@@ -88,19 +93,19 @@ internal static bool RecordMustBePassedByReference(this CXCursor cursor)
             }
         }
 
-        internal static bool MustBePassedByReference(this RecordDecl record)
-            => record.Handle.RecordMustBePassedByReference();
+        internal static bool MustBePassedByReference(this RecordDecl record, bool isForInstanceMethodReturnValue)
+            => record.Handle.RecordMustBePassedByReference(isForInstanceMethodReturnValue);
 
-        public static bool MustBePassedByReference(this ClangType type)
+        public static bool MustBePassedByReference(this ClangType type, bool isForInstanceMethodReturnValue)
         {
             switch (type)
             {
                 case ElaboratedType elaboratedType:
-                    return elaboratedType.NamedType.MustBePassedByReference();
+                    return elaboratedType.NamedType.MustBePassedByReference(isForInstanceMethodReturnValue);
                 case TypedefType typedefType:
-                    return typedefType.CanonicalType.MustBePassedByReference();
+                    return typedefType.CanonicalType.MustBePassedByReference(isForInstanceMethodReturnValue);
                 case RecordType recordType:
-                    return ((RecordDecl)recordType.Decl).MustBePassedByReference();
+                    return ((RecordDecl)recordType.Decl).MustBePassedByReference(isForInstanceMethodReturnValue);
                 default:
                     return false;
             }

Tests/Biohazrd.Tests.Common/AssertExtensions.cs

@@ -7,5 +7,16 @@ partial class Assert
         public static void ReferenceEqual<T>(T? expected, T? actual)
             where T : class
             => Equal<T>(expected, actual, ReferenceEqualityComparer.Instance);
+
+        public static void NotReferenceEqual<T>(T? expected, T? actual)
+            where T : class
+            => NotEqual<T>(expected, actual, ReferenceEqualityComparer.Instance);
+
+        public static T NotNull<T>(T? obj)
+            where T : class
+        {
+            NotNull((object?)obj);
+            return obj!;
+        }
     }
 }

Tests/Biohazrd.Tests/ReturnBufferTests.cs

@@ -0,0 +1,56 @@
+using Biohazrd.Tests.Common;
+using Xunit;
+
+namespace Biohazrd.Tests
+{
+    public sealed class ReturnBufferTests : BiohazrdTestBase
+    {
+        [Fact]
+        [RelatedIssue("https://github.com/InfectedLibraries/Biohazrd/issues/142")]
+        public void EnregisterableRecord_Windows()
+        {
+            TranslatedLibrary library = CreateLibrary
+            (@"
+struct MyInt
+{
+    int X;
+};
+
+MyInt GlobalFunction();
+
+class MyClass
+{
+public:
+    static MyInt StaticMethod();
+    MyInt InstanceMethod();
+    virtual MyInt VirtualMethod();
+};
+"
+            );
+
+            TranslatedFunction globalFunction = library.FindDeclaration<TranslatedFunction>("GlobalFunction");
+            TranslatedRecord myClass = library.FindDeclaration<TranslatedRecord>("MyClass");
+            TranslatedFunction staticMethod = myClass.FindDeclaration<TranslatedFunction>("StaticMethod");
+            TranslatedFunction instanceMethod = myClass.FindDeclaration<TranslatedFunction>("InstanceMethod");
+            TranslatedFunction virtualMethod = myClass.FindDeclaration<TranslatedFunction>("VirtualMethod");
+            TranslatedVTable vTable = Assert.NotNull(myClass.VTable);
+            TranslatedVTableEntry vTableFunction = Assert.Single(vTable.Entries, e => e.IsFunctionPointer);
+            FunctionPointerTypeReference vTableFunctionPointerType = Assert.IsType<FunctionPointerTypeReference>(vTableFunction.Type);
+
+            // https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention?view=msvc-160#return-values
+            // "User-defined types can be returned by value from global functions and static member functions."
+            // Note the lack of instance methods in this list.
+            // See https://github.com/InfectedLibraries/Biohazrd/issues/142 for details.
+            Assert.False(globalFunction.ReturnByReference);
+            Assert.False(staticMethod.ReturnByReference);
+            Assert.True(instanceMethod.ReturnByReference);
+            Assert.True(virtualMethod.ReturnByReference);
+
+            // VTable entries don't directly expose that they're return by reference.
+            // However, you can observe that they are as a side effect of the return type becoming a pointer and being added as a second parameter.
+            Assert.IsType<PointerTypeReference>(vTableFunctionPointerType.ReturnType);
+            Assert.Equal(2, vTableFunctionPointerType.ParameterTypes.Length); // (this, retbuf)
+            Assert.Equal(vTableFunctionPointerType.ReturnType, vTableFunctionPointerType.ParameterTypes[1]);
+        }
+    }
+}