[PATCH] D109192: Support: introduce public API annotation support

Fri Jun 23 01:44:34 PDT 2023

nikic added a comment.
Herald added a subscriber: wangpc.

In D109192#4407274 <https://reviews.llvm.org/D109192#4407274>, @tstellar wrote:

> After talking with @compnerd on Discord, I'm planning on helping to push this
> patch through.  At this point my goal is just to annotate the ABI functions and
> not make any changes to how -DBUILD_SHARED_LIBS=ON or -DLLVM_LINK_LLVM_DYLIB=ON
> behave.  We can make changes to those configurations in a future patch (if we
> want to).
>
> My plan is to:
>
> 1. Annotate all the ABI functions in the LLVM support library with LLVM_SUPPORT_ABI macro (this patch), and use this an example for how to do the rest of the annotations.
> 2. Annotate the rest of the LLVM ABI functions using a different macro (e.g. LLVM_ABI)

Why do we need/want to annotate the support library with a different macro from everything else? Isn't LLVMSupport part of libLLVM just like the rest?

We already have the LLVM_EXTERNAL_VISIBILITY and LLVM_LIBRARY_VISIBILITY macros. LLVM_EXTERNAL_VISIBILITY is used in the places that already use the `-fvisibility=hidden` default, namely the backend. LLVM_LIBRARY_VISIBILTY is used in a few places that want to explicitly exclude symbols in code that still uses default visibilty.

I would have expected that this would annotate symbols using the existing LLVM_EXTERNAL_VISIBILITY macro (and LLVM_LIBRARY_VISIBILITY could be retired once everything uses `-fvisibility=hidden`).

================
Comment at: llvm/include/llvm/Support/LLVMSupportExports.h:13
+#if defined(__ELF__)
+# if defined(LLVM_SUPPORT_STATIC)
+#   define LLVM_SUPPORT_ABI
----------------
What is the LLVM_SUPPORT_STATIC variable for? I don't see it defined anywhere.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109192/new/

https://reviews.llvm.org/D109192