[PATCH] D109192: [WIP/DNM] Support: introduce public API annotation support

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added inline comments.


================
Comment at: llvm/include/llvm/Support/LLVMSupportExports.h:12
+
+#if defined(__ELF__)
+# if defined(LLVM_SUPPORT_STATIC)
----------------
compnerd wrote:
> MaskRay wrote:
> > For flexibility, maybe give user a choice to redefine `LLVM_*_ABI`. Some users may want to customize this to hidden to enable better internalization (LTO,DCE) and linker GC.
> Wait, we want to give the user the flexibility to *not* define that?  I intend to make `-fvisibility=hidden` the only option once the annotations are complete.  I want to simply globally set:
> 
> ```
> set(CMAKE_C_VISIBILITY_PRESET hidden)
> set(CMAKE_CXX_VISIBILITY_PRESET hidden)
> set(CMAKE_C_VISIBILITY_INLINES_HIDDEN YES)
> set(CMAKE_CXX_VISIBILITY_INLINES_HIDDEN YES)
> ```
> 
> Everything in LLVM should be internalized unless you are doing a shared library build, where the annotations will expose the necessary interfaces.
> 
> Is there a use case that I am not considering?
If we need to add this for every library, it seems like the header should be generated?

Maybe we could even re-use https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109192