Skip to content

Conversation

philnik777
Copy link
Contributor

@philnik777 philnik777 commented Mar 27, 2025

This allows us to remove the need for _LIBCPP_TEMPLATE_VIS and fixes a bunch of missing annotations for RTTI when used across dylib boundaries. _LIBCPP_TEMPLATE_VIS itself will be removed in a separate patch, since it touches a lot of code.

This patch is a no-op for Clang. Only GCC is affected.

@philnik777 philnik777 requested a review from a team as a code owner March 27, 2025 11:03
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 27, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 27, 2025

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

This allows us to remove the need for _LIBCPP_TEMPLATE_VIS and fixes a bunch of missing annotations for RTTI when used across dylib boundaries.


Full diff: https://github.com/llvm/llvm-project/pull/133233.diff

2 Files Affected:

  • (modified) libcxx/docs/ReleaseNotes/21.rst (+4)
  • (modified) libcxx/include/__config (+9-10)
diff --git a/libcxx/docs/ReleaseNotes/21.rst b/libcxx/docs/ReleaseNotes/21.rst
index 877aa06f8b7e4..c1adb8b7c114b 100644
--- a/libcxx/docs/ReleaseNotes/21.rst
+++ b/libcxx/docs/ReleaseNotes/21.rst
@@ -60,6 +60,10 @@ Improvements and New Features
 
 - Updated formatting library to Unicode 16.0.0.
 
+- When using GCC, the ``std`` namespace is now annotated with ``[[gnu::visibility("default")]]``. This may cause changes
+  in the symbols exported from shared libraries when building with ``-fvisibility=hidden``. This also fixes RTTI
+  comparison between shared libraries, since all RTTI has the correct visibility now.
+
 Deprecations and Removals
 -------------------------
 
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 070298301b0d3..3899016847803 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -391,7 +391,7 @@ typedef __char32_t char32_t;
 #    define _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 #    define _LIBCPP_TEMPLATE_VIS
 #    define _LIBCPP_TEMPLATE_DATA_VIS
-#    define _LIBCPP_TYPE_VISIBILITY_DEFAULT
+#    define _LIBCPP_NAMESPACE_VISIBILITY
 
 #  else
 
@@ -419,17 +419,16 @@ typedef __char32_t char32_t;
 #      define _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 #    endif
 
-// GCC doesn't support the type_visibility attribute, so we have to keep the visibility attribute on templates
-#    if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS) && !__has_attribute(__type_visibility__)
-#      define _LIBCPP_TEMPLATE_VIS __attribute__((__visibility__("default")))
-#    else
-#      define _LIBCPP_TEMPLATE_VIS
-#    endif
+// This is kept to avoid a huge library-wide diff in the first step.
+// TODO: Remove this in a follow-up patch
+#    define _LIBCPP_TEMPLATE_VIS
 
 #    if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS) && __has_attribute(__type_visibility__)
-#      define _LIBCPP_TYPE_VISIBILITY_DEFAULT __attribute__((__type_visibility__("default")))
+#      define _LIBCPP_NAMESPACE_VISIBILITY __attribute__((__type_visibility__("default")))
+#    elif defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
+#      define _LIBCPP_NAMESPACE_VISIBILITY __attribute__((__visibility__("default")))
 #    else
-#      define _LIBCPP_TYPE_VISIBILITY_DEFAULT
+#      define _LIBCPP_NAMESPACE_VISIBILITY
 #    endif
 
 #  endif // defined(_LIBCPP_OBJECT_FORMAT_COFF)
@@ -588,7 +587,7 @@ typedef __char32_t char32_t;
 // If it's not clear whether using the unversioned namespace is the correct thing to do, it's not. The versioned
 // namespace (_LIBCPP_BEGIN_NAMESPACE_STD) should almost always be used.
 #  define _LIBCPP_BEGIN_UNVERSIONED_NAMESPACE_STD                                                                      \
-    _LIBCPP_PUSH_EXTENSION_DIAGNOSTICS namespace _LIBCPP_TYPE_VISIBILITY_DEFAULT std {
+    _LIBCPP_PUSH_EXTENSION_DIAGNOSTICS namespace _LIBCPP_NAMESPACE_VISIBILITY std {
 
 #  define _LIBCPP_END_UNVERSIONED_NAMESPACE_STD } _LIBCPP_POP_EXTENSION_DIAGNOSTICS
 

@philnik777 philnik777 force-pushed the gcc_namespace_visibility branch from 5bde900 to fdd2d69 Compare March 27, 2025 13:34
@philnik777 philnik777 force-pushed the gcc_namespace_visibility branch from fdd2d69 to 32039f1 Compare April 2, 2025 20:12
@philnik777 philnik777 merged commit c59d3a2 into llvm:main Apr 2, 2025
13 of 19 checks passed
@philnik777 philnik777 deleted the gcc_namespace_visibility branch April 2, 2025 20:13
philnik777 added a commit that referenced this pull request Apr 9, 2025
The need for `_LIBCPP_TEMPLATE_VIS` has been removed in #133233.
AllinLeeYL pushed a commit to AllinLeeYL/llvm-project that referenced this pull request Apr 10, 2025
The need for `_LIBCPP_TEMPLATE_VIS` has been removed in llvm#133233.
H-G-Hristov added a commit to H-G-Hristov/llvm-project that referenced this pull request May 16, 2025
H-G-Hristov added a commit to H-G-Hristov/llvm-project that referenced this pull request May 16, 2025
H-G-Hristov added a commit to H-G-Hristov/llvm-project that referenced this pull request May 17, 2025
H-G-Hristov added a commit to H-G-Hristov/llvm-project that referenced this pull request Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants