[PATCH] D101972: Do not set CMAKE_CXX_VISIBILITY_PRESET to hidden

[Tom Stellard via Phabricator via llvm-commits]

tstellar added a comment.

@rnk I agree with

In D101972#2742231 <https://reviews.llvm.org/D101972#2742231>, @rnk wrote:

>> As an example, llvm::Any::TypeId::Id relies on the uniqueness of the address of a
>> static variable defined in a template function.
>
> I just read that line, and I'm thinking to myself, obviously that won't work, don't do that, it doesn't work with DLLs, it doesn't work with hidden visibility. I *just* helped prepare a talk explaining that this code pattern isn't portable. Ideally, I would like to move in the direction of compiling *more* of LLVM with hidden visibility, not less. LLVM shared libraries already have far too many dynamic symbols. I believe Lang Hames attempted to use this code pattern in the past to refactor the pass ID into a template base class, and we reverted it for the same reasons. I wasn't able to quickly find the patch for reference.
>
> What code actually uses llvm::Any? I only see 8 includes of Any.h across the codebase. It looks like @zturner added it in 2018 for use in unit tests. Could we remove it? If we do need it, surely the standard C++17 implementation of std::any supports compiling with fvisibility=hidden, so could we just use whatever solution the STL uses? Maybe just put default visibility on the typeid global?

I also would really like to see more of LLVM with hidden visibility too.  My main concern with this issue is that without some kind of enforcement in place, this same code pattern is going to get reintroduced somewhere else in the codebase. and debugging this issue was very costly.  Do you have any ideas for how we might be able to ensure this pattern doesn't re-emerge?


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

https://reviews.llvm.org/D101972