[libc-commits] [libc] [libc] Set default visibility for LLVM functions (PR #116686)

[Nick Desaulniers via libc-commits]

nickdesaulniers wrote:

> FWIW LLVM_LIBC_FUNCTION_ATTR is used to set the default visibility in the default Bazel configuration as well:

So now that's three cases where downstream users are using `LLVM_LIBC_FUNCTION_ATTR` just to set the non-namespaced symbols to default visibility! :vomiting_face: 
1. [Fuchsia](https://cs.opensource.google/fuchsia/fuchsia/+/main:zircon/system/ulib/c/BUILD.gn;l=184;drc=4cd8bba05cb37d30da9eb365b369ed0a0bb43267)
2. [Android](https://cs.android.com/android/platform/superproject/main/+/main:external/llvm-libc/Android.bp;l=41)
3. [Bazel](https://github.com/llvm/llvm-project/blob/6626ed6f9fae79d35aba504f50bac4375686a03b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl#L114-L120)

(Bazel is also using LLVM_LIBC_FUNCTION_ATTR to add weak linkage attributes, but I think that can be better solved by @lntue's https://github.com/llvm/llvm-project/pull/116160).

So I think we should:
1. always mark the non-namespaced symbols as default visibility. (See diff below)
2. remove LLVM_LIBC_FUNCTION_ATTR, as there don't appear to be any downstream use cases other than fixing the visibility, which is better addressed by 1 above. Maybe a use case comes back in the future, at which point I'm happy to revisit adding it.

My suggestion for 1 above would look like:
```diff
diff --git a/libc/src/__support/common.h b/libc/src/__support/common.h
index 48c773fa02c1..45ded79f061a 100644
--- a/libc/src/__support/common.h
+++ b/libc/src/__support/common.h
@@ -24,6 +24,7 @@
 // MacOS needs to be excluded because it does not support aliasing.
 #if defined(LIBC_COPT_PUBLIC_PACKAGING) && (!defined(__APPLE__))
 #define LLVM_LIBC_FUNCTION_IMPL(type, name, arglist)                           \
+  [[gnu::visibility("default")]]                                               \
   LLVM_LIBC_FUNCTION_ATTR decltype(LIBC_NAMESPACE::name)                       \
       __##name##_impl__ __asm__(#name);                                        \
   decltype(LIBC_NAMESPACE::name) name [[gnu::alias(#name)]];                   \
```

My suggestion for 2 (stacked on top of 1) would look like:
```diff
diff --git a/libc/src/__support/common.h b/libc/src/__support/common.h
index 45ded79f061a..5bb08c6a1195 100644
--- a/libc/src/__support/common.h
+++ b/libc/src/__support/common.h
@@ -17,16 +17,11 @@
 #include "src/__support/macros/config.h"
 #include "src/__support/macros/properties/architectures.h"
 
-#ifndef LLVM_LIBC_FUNCTION_ATTR
-#define LLVM_LIBC_FUNCTION_ATTR
-#endif
-
 // MacOS needs to be excluded because it does not support aliasing.
 #if defined(LIBC_COPT_PUBLIC_PACKAGING) && (!defined(__APPLE__))
 #define LLVM_LIBC_FUNCTION_IMPL(type, name, arglist)                           \
   [[gnu::visibility("default")]]                                               \
-  LLVM_LIBC_FUNCTION_ATTR decltype(LIBC_NAMESPACE::name)                       \
-      __##name##_impl__ __asm__(#name);                                        \
+  decltype(LIBC_NAMESPACE::name) __##name##_impl__ __asm__(#name);             \
   decltype(LIBC_NAMESPACE::name) name [[gnu::alias(#name)]];                   \
   type __##name##_impl__ arglist
 #else
```
(There's a few other mentions of `LLVM_LIBC_FUNCTION_ATTR` in comments/docs to clean up, too.  utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl would have to get cleaned up, too).

Thoughts?

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