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

Sat Sep 4 13:45:38 PDT 2021

compnerd marked 3 inline comments as done.
compnerd added inline comments.

================
Comment at: llvm/include/llvm/Support/LLVMSupportExports.h:12
+
+#if defined(__ELF__)
+# if defined(LLVM_SUPPORT_STATIC)
----------------
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?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109192