[PATCH] D34637: [libunwind] Add _LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added inline comments.


================
Comment at: src/config.h:47
 
-// FIXME: these macros are not correct for COFF targets
-#define _LIBUNWIND_EXPORT __attribute__((visibility("default")))
-#define _LIBUNWIND_HIDDEN __attribute__((visibility("hidden")))
+#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+  #define _LIBUNWIND_EXPORT
----------------
thomasanderson wrote:
> thomasanderson wrote:
> > compnerd wrote:
> > > If the intent is to prevent the symbols from being exported, then hidden visibility is still desirable.  I think that having something like a check for a static build makes more sense rather than just disabling the visibility annotations.
> > This is what libc++ and libc++abi do already.  Search for _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS and _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS
> Forgot to say, for this use case, you're expected to link with -fvisibility=hidden.
I assume you mean compile libunwind with `-fvisibility=hidden`, which makes the condition even weirder, because you really do want the visibility, just dont want to mark functions for export (basically, you want to use the visibility similar to DLL storage).  Indicating a static build and conditionalizing it on that seems more inline with the intent and the desired behaviour.


https://reviews.llvm.org/D34637