[libc-commits] [libc] [libc] Add LIBC_NAMESPACE_HIDDEN_DECL macro (PR #97109)

[Roland McGrath via libc-commits]

frobtech wrote:

> > > I'm thinking about moving all of our internal implementation to something like `#define LIBC_NAMESPACE_INTERNAL LIBC_NAMESPACE::internal` and make that has hidden visibility by default. We will only leave entry point functions in `LIBC_NAMESPACE`. WDYT?
> > > tagging @frobtech @brooksmoses
> > 
> > 
> > I don't think we need to bother, since we already have a macro that marks all of our "API" functions right? (Maybe errno would need something special?). Figured it's sufficient to make everything hidden except our API functions.
> 
> `LIBC_NAMESPACE` is already the internal implementation namespace and the attribute has no affect on how the public C symbols get defined. Those are all controlled by `LLVM_LIBC_FUNCTION_ATTR` I think.

That's correct.  Using `-fvisibility=hidden` or not is a no-op once we ensure that we're using `LIBC_NAMESPACE_DECL` uniformly around everything.  It's already the case that embedders who want the public ABI symbols to have a specific visibility set `LLVM_LIBC_FUNCTION_ATTR` for that purpose.  We do need a macro that's just like `LLVM_LIBC_FUNCTION` but for variables, and it might as well use the same rather than a new configuration macro, just rename it to `LLVM_LIBC_PUBLIC_ATTR` unless we envision a reason to use it for something that should differ between functions and variables.

It's a good normal practice to use `-fvisibility=hidden` across the board, so there's nothing wrong with doing that in cmake.  The non-cmake builds using the llvm-libc code are all compiling it that way already (since they apply that to everything in those builds already).  But we should not be relying on that, because we need these namespace attributes to ensure that the extern declarations in a given TU have known visibility to the codegen and don't generate GOT indirections.  If we're doing that correctly everywhere, then there will be no part of our code that comes out any different due to `-fvisibility=hidden` anyway.

https://github.com/llvm/llvm-project/pull/97109