[llvm] add LLVM_ABI_FRIEND macro for friend function decls

[andrurogerz]

Purpose

Introduce a new LLVM_ABI_FRIEND macro to llvm/Support/Compiler.h for annotating friend function declarations for DLL export.

Overview

1. Add a new LLVM_ABI_FRIEND macro, which behaves identically to the existing LLVM_ABI macro on Windows and compiles to nothing on other platforms.
2. Update existing documentation to describe proper usage of the LLVM_ABI_FRIEND annotation.

Background

• MSVC issues a warning when it encounters a friend function declaration that does not match the DLL import/export annotation of the original function.
• When compiling ELF and Mach-O shared libraries, friend function declarations with visibility annotations produce compilation errors (GCC) and warnings (Clang).
• Additional context on the effort to annotate LLVM's public interface is in this discourse.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Andrew Rogers (andrurogerz)

Changes

Purpose

Introduce a new LLVM_FRIEND_ABI macro to llvm/Support/Compiler.h for annotating friend function declarations for DLL export.

Overview

1. Add a new LLVM_FRIEND_ABI macro, which behaves identically to the existing LLVM_ABI macro on Windows and compiles to nothing on other platforms.
2. Update existing documentation to describe proper usage of the LLVM_FRIEND_ABI annotation.

Background

• MSVC issues a warning when it encounters a friend function declaration that does not match the DLL import/export annotation of the original function.
• When compiling ELF and Mach-O shared libraries, friend function declarations with visibility annotations produce compilation errors (GCC) and warnings (Clang).
• Additional context on the effort to annotate LLVM's public interface is in this discourse.

---

Full diff: https://github.com/llvm/llvm-project/pull/136595.diff

2 Files Affected:

• (modified) llvm/docs/InterfaceExportAnnotations.rst (+7-6)
• (modified) llvm/include/llvm/Support/Compiler.h (+10)

diff --git a/llvm/docs/InterfaceExportAnnotations.rst b/llvm/docs/InterfaceExportAnnotations.rst
index eecf6ffe6eaca..715472f644763 100644
--- a/llvm/docs/InterfaceExportAnnotations.rst
+++ b/llvm/docs/InterfaceExportAnnotations.rst
@@ -222,9 +222,10 @@ method in a C++ class, it may be annotated for export.
 
 Friend Functions
 ~~~~~~~~~~~~~~~~
-Friend functions declared in a class, struct or union must be annotated with
-``LLVM_ABI`` if the corresponding function declaration is also annotated. This
-requirement applies even when the class itself is annotated with ``LLVM_ABI``.
+Friend functions declared in a class, struct or union should be annotated with
+``LLVM_FRIEND_ABI`` if the corresponding function declaration is annotated with
+``LLVM_ABI``. This requirement applies even when the class containing the friend
+declaration is annotated with ``LLVM_ABI``.
 
 .. code:: cpp
 
@@ -236,14 +237,14 @@ requirement applies even when the class itself is annotated with ``LLVM_ABI``.
    class ExampleClass {
      // Friend declaration of a function must be annotated the same as the actual
      // function declaration.
-     LLVM_ABI friend int friend_function(ExampleClass &obj);
+     LLVM_FRIEND_ABI friend int friend_function(ExampleClass &obj);
    };
 
 .. note::
 
    Annotating the friend declaration avoids an “inconsistent dll linkage”
-   compiler error when building for Windows. This annotation is harmless but not
-   required when building ELF or Mach-O shared libraries.
+   compiler error when building a DLL for Windows. The ``LLVM_FRIEND_ABI``
+   annotation is a no-op when building ELF or Mach-O shared libraries.
 
 Virtual Table and Type Info
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/llvm/include/llvm/Support/Compiler.h b/llvm/include/llvm/Support/Compiler.h
index d265d864228ca..0f29299a1f0b8 100644
--- a/llvm/include/llvm/Support/Compiler.h
+++ b/llvm/include/llvm/Support/Compiler.h
@@ -167,6 +167,11 @@
 /// for both functions and classes. On windows its turned in to dllimport for
 /// library consumers, for other platforms its a default visibility attribute.
 ///
+/// LLVM_FRIEND_ABI is for annotating friend function declarations when the
+/// target function's original declaration is annotated with LLVM_ABI. This
+/// macro matches the LLVM_ABI macro on Windows, on other platforms it does
+/// nothing.
+///
 /// LLVM_C_ABI is used to annotated functions and data that need to be exported
 /// for the libllvm-c API. This used both for the llvm-c headers and for the
 /// functions declared in the different Target's c++ source files that don't
@@ -183,6 +188,7 @@
 // missing symbol linker errors on windows.
 #if defined(LLVM_BUILD_STATIC)
 #define LLVM_ABI
+#define LLVM_FRIEND_ABI
 #define LLVM_TEMPLATE_ABI
 #define LLVM_EXPORT_TEMPLATE
 #define LLVM_ABI_EXPORT
@@ -196,21 +202,25 @@
 #define LLVM_TEMPLATE_ABI __declspec(dllimport)
 #define LLVM_EXPORT_TEMPLATE
 #endif
+#define LLVM_FRIEND_ABI LLVM_ABI
 #define LLVM_ABI_EXPORT __declspec(dllexport)
 #elif defined(__ELF__) || defined(__MINGW32__) || defined(_AIX) ||             \
     defined(__MVS__)
 #define LLVM_ABI LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
+#define LLVM_FRIEND_ABI
 #define LLVM_TEMPLATE_ABI LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
 #define LLVM_EXPORT_TEMPLATE
 #define LLVM_ABI_EXPORT LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
 #elif defined(__MACH__) || defined(__WASM__) || defined(__EMSCRIPTEN__)
 #define LLVM_ABI LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
+#define LLVM_FRIEND_ABI
 #define LLVM_TEMPLATE_ABI
 #define LLVM_EXPORT_TEMPLATE
 #define LLVM_ABI_EXPORT LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
 #endif
 #else
 #define LLVM_ABI
+#define LLVM_FRIEND_ABI
 #define LLVM_TEMPLATE_ABI
 #define LLVM_EXPORT_TEMPLATE
 #define LLVM_ABI_EXPORT

[andrurogerz]

@compnerd this change is an alternative to using the __attribute__ style annotations for visibility in #135995. Even though it introduces a new macro, I think it is a better solution because it is less invasive and ensures we only introduce export annotations to friend functions where needed: on Windows.

There are only a couple of dozen total friend function declarations throughout LLVM, so I don't think the additional LLVM_FRIEND_ABI macro will be a maintenance burden.

[compnerd]

I think that this is fine to make progress, but we should revisit the macros. Right now, I see the following:

• LLVM_ABI
• LLVM_FRIEND_ABI
• LLVM_TEMPLATE_ABI
• LLVM_EXPORT_TEMPLATE
• LLVM_ABI_EXPORT

The problem is that this is pretty complex for someone coming to this world. I think that we should try to minimise the number of macros. *_ABI is reasonable. *_FRIEND_ABI I suppose is not something that we can get away it. BTW, would it be possible to spell that as:

friend LLVM_ABI_FRIEND void function();

The TEMPLATE_ABI and LLVM_EXPORT_TEMPLATE would be nice to merge somehow if possible.

I don't understand the reason for LLVM_ABI_EXPORT, we should be able to use LLVM_ABI for that no?

[andrurogerz]

BTW, would it be possible to spell that as LLVM_ABI_FRIEND

Sure, I will do that.

I don't understand the reason for LLVM_ABI_EXPORT, we should be able to use LLVM_ABI for that no?

I can't find any instances of LLVM_ABI_EXPORT in the source tree. I was initially hesitant to remove it because of the comment in this file, but I never encountered a need for it. I'll go ahead remove it.

The TEMPLATE_ABI and LLVM_EXPORT_TEMPLATE would be nice to merge somehow if possible.

They can't be merged. We may be able to get rid of LLVM_EXPORT_TEMPLATE and just use LLVM_ABI in its place. I will take another look at that one.. However, we do need an "import only" macro for extern template instance declarations because MSVC warns when you decorate any extern template declaration for export with __declspec(dllexport) (dllimport is fine). That's what LLVM_TEMPLATE_ABI is used for. We could rename it LLVM_ABI_IMPORT, but since the only case I found it required is for extern template instance declarations, I suppose its current name makes sense.

For the most part, people will just need to know LLVM_ABI and everything else is fairly special-case.