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

[via libc-commits]

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

>From afb5d2fb44dba9830446b361fb54841489e2632d Mon Sep 17 00:00:00 2001
From: Leonard Chan <leonardchan at google.com>
Date: Fri, 28 Jun 2024 13:27:20 -0700
Subject: [PATCH] [libc] Add LIBC_NAMESPACE_DECL macro

This defines to LIBC_NAMESPACE with
`__attribute__((visibility("hidden")))` so all the symbols under it have
hidden visibility. This new macro should be used when declaring a new
namespace that will have internal functions/globals and LIBC_NAMESPACE
should be used as a means of accessing functions/globals declared within
LIBC_NAMESPACE_DECL.
---
 libc/docs/dev/clang_tidy_checks.rst       |  5 ++++
 libc/docs/dev/code_style.rst              | 31 +++++++++++++++++++++++
 libc/docs/dev/implementation_standard.rst | 12 ++++-----
 libc/src/__support/macros/config.h        | 10 ++++++++
 4 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/libc/docs/dev/clang_tidy_checks.rst b/libc/docs/dev/clang_tidy_checks.rst
index 3feb5375ef113..198d8f16d1cd7 100644
--- a/libc/docs/dev/clang_tidy_checks.rst
+++ b/libc/docs/dev/clang_tidy_checks.rst
@@ -64,6 +64,11 @@ This check that ensures any function call resolves to a function within the
         void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
     }
 
+..
+  TODO(97655): The clang-tidy check should be updated to ensure the namespace
+  declaration uses LIBC_NAMESPACE_DECL as opposed to LIBC_NAMESPACE. The former
+  should be used for accessing globals in LIBC_NAMESPACE rather than declaration.
+
 
 callee-namespace
 ----------------
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index 170ef6598a9d8..28d8277417b2b 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -260,3 +260,34 @@ Patches containing any amount of Assembly ideally should be approved by 2
 maintainers. llvm-libc maintainers reserve the right to reject Assembly
 contributions that they feel could be better maintained if rewritten in C++,
 and to revisit this policy in the future.
+
+LIBC_NAMESPACE_DECL
+===================
+
+llvm-libc provides a macro `LIBC_NAMESPACE` which contains internal implementations of
+libc functions and globals. This macro should only be used as an
+identifier for accessing such symbols within the namespace (like `LIBC_NAMESPACE::cpp::max`).
+Any usage of this namespace for declaring or defining internal symbols should
+instead use `LIBC_NAMESPACE_DECL` which declares `LIBC_NAMESPACE` with hidden visibility.
+
+Example usage:
+
+.. code-block:: c++
+
+   #include "src/__support/macros/config.h"  // The macro is defined here.
+
+   namespace LIBC_NAMESPACE_DECL {
+
+   void new_function() {
+     ...
+   }
+
+   }  // LIBC_NAMESPACE_DECL
+
+Having hidden visibility on the namespace ensures extern declarations in a given TU
+have known visibility and never generate GOT indirextions. The attribute guarantees
+this independently of global compile options and build systems.
+
+..
+  TODO(97655): We should have a clang-tidy check to enforce this and a
+  fixit implementation.
diff --git a/libc/docs/dev/implementation_standard.rst b/libc/docs/dev/implementation_standard.rst
index 790c06726d6b5..277fee9c19089 100644
--- a/libc/docs/dev/implementation_standard.rst
+++ b/libc/docs/dev/implementation_standard.rst
@@ -26,17 +26,17 @@ example. The ``isalpha`` function will be declared in an internal header file
     #ifndef LLVM_LIBC_SRC_CTYPE_ISALPHA_H
     #define LLVM_LIBC_SRC_CTYPE_ISALPHA_H
 
-    namespace LIBC_NAMESPACE {
+    namespace LIBC_NAMESPACE_DECL {
 
     int isalpha(int c);
 
-    } // namespace LIBC_NAMESPACE
+    } // namespace LIBC_NAMESPACE_DECL
 
     #endif LLVM_LIBC_SRC_CTYPE_ISALPHA_H
 
 Notice that the ``isalpha`` function declaration is nested inside the namespace
-``LIBC_NAMESPACE``. All implementation constructs in LLVM-libc are declared
-within the namespace ``LIBC_NAMESPACE``.
+``LIBC_NAMESPACE_DECL``. All implementation constructs in LLVM-libc are declared
+within the namespace ``LIBC_NAMESPACE_DECL``.
 
 ``.cpp`` File Structure
 -----------------------
@@ -49,13 +49,13 @@ which must be defined with the ``LLVM_LIBC_FUNCTION`` macro. For example, the
 
     // --- isalpha.cpp --- //
 
-    namespace LIBC_NAMESPACE {
+    namespace LIBC_NAMESPACE_DECL {
 
     LLVM_LIBC_FUNCTION(int, isalpha, (int c)) {
       // ... implementation goes here.
     }
 
-    } // namespace LIBC_NAMESPACE
+    } // namespace LIBC_NAMESPACE_DECL
 
 Notice the use of the macro ``LLVM_LIBC_FUNCTION``. This macro helps us define
 a C alias symbol for the C++ implementation. For example, for a library build,
diff --git a/libc/src/__support/macros/config.h b/libc/src/__support/macros/config.h
index 6390c7992325d..3da6b8bc94c8b 100644
--- a/libc/src/__support/macros/config.h
+++ b/libc/src/__support/macros/config.h
@@ -27,4 +27,14 @@
 #define LIBC_HAS_FEATURE(f) 0
 #endif
 
+// Declare a LIBC_NAMESPACE with hidden visibility. `namespace
+// LIBC_NAMESPACE_DECL {` should be used around all declarations and definitions
+// for libc internals as opposed to just `namespace LIBC_NAMESPACE {`. This
+// ensures that all declarations within this namespace have hidden
+// visibility, which optimizes codegen for uses of symbols defined in other
+// translation units in ways that can be necessary for correctness by avoiding
+// dynamic relocations. This does not affect the public C symbols which are
+// controlled independently via `LLVM_LIBC_FUNCTION_ATTR`.
+#define LIBC_NAMESPACE_DECL [[gnu::visibility("hidden")]] LIBC_NAMESPACE
+
 #endif // LLVM_LIBC_SRC___SUPPORT_MACROS_CONFIG_H