[libc-commits] [PATCH] D139678: [libc][bazel] Mark all libc public functions to have "default" visibility.

[Guillaume Chatelet via Phabricator via libc-commits]

gchatelet added inline comments.


================
Comment at: libc/src/__support/common.h:25
 #ifndef LLVM_LIBC_FUNCTION_ATTR
-#define LLVM_LIBC_FUNCTION_ATTR
+#define LLVM_LIBC_FUNCTION_ATTR __attribute__((visibility("default")))
 #endif
----------------
Since we're carefully defining it in the build system I think it makes sense to err if the attribute is not defined otherwise it's complicated to reason about it and debug.
```
#error make sure that LLVM_LIBC_FUNCTION_ATTR is defined in either CMake of Bazel configuration
```

Then we'd need the same logic in CMake.


================
Comment at: utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl:51
       srcs: The .cpp files which contain the function implementation.
-      weak: Whether the symbol is marked weak.
+      weak: Make the symbol corresponding to the libc function "weak".
       deps: The list of target dependencies if any.
----------------
Thx!


================
Comment at: utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl:81
+        default_vis = "__attribute__((visibility(\"default\")))"
+        copts.append("-DLLVM_LIBC_FUNCTION_ATTR='%s %s'" % (weak_attr, default_vis))
+    _libc_library(
----------------
This should be a `defines` attribute I think (and above as well)
https://bazel.build/reference/be/c-cpp#cc_binary.defines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139678