[PATCH] D113610: Only set LLVM_EXTERNAL_VISIBILITY when building libLLVM dylib

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: llvm/include/llvm/Support/Compiler.h:133
+#define LLVM_EXTERNAL_VISIBILITY
+#endif
 #else
----------------
If I understand correctly, the `-DBUILD_SHARED_LIBS=YES` will use this "else" and not define LLVM_EXTERNAL_VISIBILITY.
I'm a bit puzzled why is this correct?
I would expect that `BUILD_SHARED_LIBS` requires at least as much visibility as `LLVM_BUILD_LLVM_DYLIB` (more in some cases).

That is:  AFAIU we're making the unit of visibility smaller with `BUILD_SHARED_LIBS` (many small shared object) compared to `LLVM_BUILD_LLVM_DYLIB`: if a symbol `foo` from libA is used in libB, it does not need to be externally visible if we link the static libA and libB together in a single dylib, but if libA and libB are each individual dylibs then `libA` needs to make `foo` visible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113610