Biohazrd icon indicating copy to clipboard operation
Biohazrd copied to clipboard

Explore alternative force-export method for inline methods using forward declarations

Open PathogenDavid opened this issue 3 years ago • 4 comments

While adding a workaround for https://github.com/ocornut/imgui/pull/3850 I stumbled upon an alternative solution for (potentially?) forcing inline methods to export. Turns out you can add __declspec(dllexport) to a forward declaration and the members will be exported. (See here)

However, I did not test to see if this worked for inline methods.

I seem to recall trying this with inline functions and it didn't work, but this could potentially cut down on the noise in certain libraries.

PathogenDavid avatar Feb 27 '21 20:02 PathogenDavid

Fixing this sort-of requires fixing https://github.com/InfectedLibraries/Biohazrd/issues/172

Edit: Actually, maybe not since the inline reference file is typically not part of the Biohazrd build. Either way we could special-case things just for this issue if it came to it. Fixing that issue is actually a bit of a pain to do without increasing the complexity of translation unit parsing (which is already pretty complicated.)

PathogenDavid avatar Feb 27 '21 20:02 PathogenDavid

I seem to recall trying this with inline functions and it didn't work

I messed around a little on Godbolt and I think the issue was I tried putting the inline forward-declaration after the function definition. Either that or I tried it to export a single inline method out of a class after it was defined...

Edit: Although in the case of functions this is easier said than done since you need access to all of the parameter types which quickly becomes a pain.

PathogenDavid avatar Feb 27 '21 21:02 PathogenDavid

Womp-womp. Specifying dllexport on both the struct and a method within it is illegal for whatever reason:

struct __declspec(dllexport) Hello;

struct Hello
{
    __declspec(dllexport) void Test(); // error C2487: 'Test': member of dll interface class may not be declared with dll interfac
};

PathogenDavid avatar Feb 27 '21 21:02 PathogenDavid

The good news is it did work besides that, and it exported the constructor/destructor too. Unfortunately I don't think this approach is likely to be realistic except for libraries which aren't even annotated with __declspec(dllexport) unless we can also disable all dllexport annotations and add all the nonsense required to also forward declare functions. (In my experience, libraries tend to be all-or-nothing with their exports and use a single macro for everything.)

This might be viable as an alternate mode or as a mode that is opportunistically used when none of the individual methods are marked as dllexport.

However, since this method is problematic and the current solution works pretty well as it is, I don't think I can consider this a high priority. In the case of ImGui specifically, I'm actually more inclined to offer a PR to normalize how IMGUI_API is applied to structs.

As visible in the diff below, this also requires providing a mechanism for writing to the area before includes in CppCodeWriter since the forward-declarations have to come before any definitions. Although I think in a real implementation I'd emit a separate header file that gets included from the export helper with custom sorting making it the firstest.

Also note that the below implementation is hard coded to only apply to ImDrawCmd after I discovered the double-export issue above.

diff --git a/Biohazrd.Utilities/InlineExportHelper.cs b/Biohazrd.Utilities/InlineExportHelper.cs
index df60031..231e044 100644
--- a/Biohazrd.Utilities/InlineExportHelper.cs
+++ b/Biohazrd.Utilities/InlineExportHelper.cs
@@ -8,6 +8,7 @@ using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
 using System.Runtime.InteropServices;
+using System.Text;
 using System.Threading;
 
 namespace Biohazrd.Utilities
@@ -18,11 +19,31 @@ namespace Biohazrd.Utilities
         private bool Used = false;
         private readonly List<TranslatedFunction> FunctionsExportedViaFunctionPointer = new();
         private readonly List<TranslatedFunction> FunctionsExportedViaTrampoline = new();
+        private readonly List<TranslatedRecord> RecordsExportedViaForwardDeclaration = new();
         private volatile int NextTrampolineId = 0;
-        private readonly CppCodeWriter Writer;
+        private readonly CppCodeWriterWithHax Writer;
+
+        [ProvidesOutputSessionFactory]
+        private class CppCodeWriterWithHax : CppCodeWriter
+        {
+            public StringBuilder BeforeIncludes = new();
+
+            protected CppCodeWriterWithHax(OutputSession session, string filePath)
+                : base(session, filePath)
+            { }
+
+            private static OutputSession.WriterFactory<CppCodeWriterWithHax> FactoryMethod => (session, filePath) => new CppCodeWriterWithHax(session, filePath);
+
+            protected override void WriteBetweenHeaderAndCode(StreamWriter writer)
+            {
+                writer.WriteLine(BeforeIncludes.ToString());
+
+                base.WriteBetweenHeaderAndCode(writer);
+            }
+        }
 
         public InlineExportHelper(OutputSession session, string filePath)
-            => Writer = session.Open<CppCodeWriter>(filePath);
+            => Writer = session.Open<CppCodeWriterWithHax>(filePath);
 
         public void ForceInclude(string filePath, bool systemInclude = false)
         {
@@ -85,6 +106,15 @@ namespace Biohazrd.Utilities
             return declaration;
         }
 
+        protected override TransformationResult TransformRecord(TransformationContext context, TranslatedRecord declaration)
+        {
+            if (declaration.Name == "ImDrawCmd")
+            {
+                RecordsExportedViaForwardDeclaration.Add(declaration);
+            }
+            return declaration;
+        }
+
         protected override TransformationResult TransformFunction(TransformationContext context, TranslatedFunction declaration)
         {
             // Edge case: All static non-method functions need to be exported by a trampoline (even the non-inline ones.)
@@ -106,6 +136,9 @@ namespace Biohazrd.Utilities
             if (declaration.Declaration is not FunctionDecl functionDecl)
             { return declaration; }
 
+            if (declaration.Declaration is CXXMethodDecl && context.ParentDeclaration?.Name == "ImDrawCmd")
+            { return declaration; }
+
             // Private/protected methods can't be exported using the techniques we use today
             // https://github.com/InfectedLibraries/Biohazrd/issues/162
             if (functionDecl is CXXMethodDecl methodDecl && methodDecl.Access != CX_CXXAccessSpecifier.CX_CXXPublic)
@@ -147,6 +180,11 @@ namespace Biohazrd.Utilities
         {
             const string dummyNamespaceName = "____BiohazrdInlineExportHelpers";
 
+            foreach (TranslatedRecord record in RecordsExportedViaForwardDeclaration)
+            {
+                Writer.BeforeIncludes.AppendLine($"{record.Kind.ToString().ToLowerInvariant()} __declspec(dllexport) {record.Original.Name};");
+            }
+
             //=====================================================================================
             // Handle function pointer exports
             //=====================================================================================

PathogenDavid avatar Feb 27 '21 21:02 PathogenDavid