[libc++] Introduce _LIBCPP_HIDE_FROM_ABI_NAMESPACE_STD

[philnik]

Summary

We've added _LIBCPP_HIDE_FROM_ABI to a lot of functions lately, because we now have a clang-tidy check that makes sure we add it to all the functions that need it. Because of that a lot of functions are marked [[gnu::always_inline]] with GCC, which results in GCC taking a lot longer to compile with libc++. This patch introduces _LIBCPP_HIDE_FROM_ABI_NAMESPACE, which fulfills the same purpose as adding _LIBCPP_HIDE_FROM_ABI to the functions inside it. It also avoids a lot of _LIBCPP_HIDE_FROM_ABI annotations, since it can be used for a lot of functions, instead of having to mark every single one. Structs and ABI-sensitive functions should not be placed inside this namespace, since the ABI will change with bumping the libc++ version macro. To avoid having structs inside the namespace or having functions unnecessarily marked _LIBCPP_HIDE_FROM_ABI, this patch also adds clang-tidy checks for these cases.

[Mordante]

I'm not convinced this is an improvement.
There are restrictions to what can be placed in the "namespace". It also removes the attribute from the reader. To know whether a function is hidden from the ABI you need to look at the "namespace" the object is placed in and for member functions you still need use the macro.
For the small example the readability remains the same, but in the end it doesn't save a lot of code.

Are structs/classes/unions the only exception, what about enum/enum class?

When doing this the visibly documentation needs to be updated too.

Maybe it would be good to discuss this patch on Discord.

libcxx/test/tools/clang_tidy_checks/hide_from_abi.cpp

16

__MAMIPA?

[huixie90]

With regard to gcc's always_inline, since we are using _LIBCPP_HIDE_FROM_ABI everywhere now, would it cause this problem to gcc?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104719#c10

[philnik]

In D141812#4058098, @huixie90 wrote:

With regard to gcc's always_inline, since we are using _LIBCPP_HIDE_FROM_ABI everywhere now, would it cause this problem to gcc?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104719#c10

I don't know if it errors out at any point, but code bloat at -O0 is definitely a problem.

libcxx/test/tools/clang_tidy_checks/hide_from_abi.cpp

16

major, minor, patch. Do you have a better suggestion?

[Mordante]

libcxx/test/tools/clang_tidy_checks/hide_from_abi.cpp

16

I would use the magic number 8 and comment describing why it's 8.
I initially read it as MAMI PA, which seems to have no real meaning in libc++.

[huixie90]

libcxx/include/__config

653

Does this have attribute((visibility(“hidden”))) ?

libcxx/include/__algorithm/any_of.h

 #define _LIBCPP___ALGORITHM_ANY_OF_H
 
 #include <__config>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
-_LIBCPP_BEGIN_NAMESPACE_STD
+_LIBCPP_BEGIN_HIDE_FROM_ABI_NAMESPACE_STD
 
 template <class _InputIterator, class _Predicate>
-_LIBCPP_NODISCARD_EXT inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX20 bool
+_LIBCPP_NODISCARD_EXT inline _LIBCPP_CONSTEXPR_SINCE_CXX20 bool
 any_of(_InputIterator __first, _InputIterator __last, _Predicate __pred) {
   for (; __first != __last; ++__first)
     if (__pred(*__first))
       return true;
   return false;
 }
 
-_LIBCPP_END_NAMESPACE_STD
+_LIBCPP_END_HIDE_FROM_ABI_NAMESPACE_STD
 
 #endif // _LIBCPP___ALGORITHM_ANY_OF_H

libcxx/include/__config

 #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
 
 // Inline namespaces are available in Clang/GCC/MSVC regardless of C++ dialect.
 // clang-format off
 #  define _LIBCPP_BEGIN_NAMESPACE_STD namespace std { inline namespace _LIBCPP_ABI_NAMESPACE {
 #  define _LIBCPP_END_NAMESPACE_STD }}
 #  define _VSTD std
 
+#  define _LIBCPP_BEGIN_HIDE_FROM_ABI_NAMESPACE_STD _LIBCPP_BEGIN_NAMESPACE_STD inline namespace _LIBCPP_CONCAT(__, _LIBCPP_VERSION) _LIBCPP_HIDDEN {
+#  define _LIBCPP_END_HIDE_FROM_ABI_NAMESPACE_STD _LIBCPP_END_NAMESPACE_STD }
+
 _LIBCPP_BEGIN_NAMESPACE_STD _LIBCPP_END_NAMESPACE_STD
 
 #  if _LIBCPP_STD_VER > 14
 #    define _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM                                                                         \
        _LIBCPP_BEGIN_NAMESPACE_STD inline namespace __fs { namespace filesystem {
 #  else
 #    define _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM                                                                         \
       _LIBCPP_BEGIN_NAMESPACE_STD namespace __fs { namespace filesystem {

libcxx/test/tools/clang_tidy_checks/hide_from_abi.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang-tidy/ClangTidyCheck.h"
 #include "clang-tidy/ClangTidyModuleRegistry.h"
 
 #include "hide_from_abi.hpp"
 
+namespace {
+bool is_hide_from_abi_namespace(std::string_view str) {
+  return str.size() == strlen("__MAMIPA") && str.starts_with("__");
+}
+
+AST_MATCHER(clang::Decl, isInHideFromABINamespace) {
+  auto context = Node.getDeclContext();
+  if (context == nullptr || !context->isNamespace())
+    return false;
+
+  auto namespace_decl = cast<clang::NamespaceDecl>(context);
+  while (!namespace_decl->isInline() || !is_hide_from_abi_namespace(namespace_decl->getName())) {
+    auto namespace_context = namespace_decl->getParent();
+    if (!namespace_context->isNamespace())
+      return false;
+    namespace_decl = cast<clang::NamespaceDecl>(namespace_context);
+  }
+  return true;
+}
+} // namespace
+
 namespace libcpp {
 hide_from_abi::hide_from_abi(llvm::StringRef name, clang::tidy::ClangTidyContext* context)
     : clang::tidy::ClangTidyCheck(name, context) {}
 
 void hide_from_abi::registerMatchers(clang::ast_matchers::MatchFinder* finder) {
   using namespace clang::ast_matchers;
   finder->addMatcher(
       functionDecl(
           unless(anyOf(
               hasName("__introsort"),
               hasName("__inplace_merge"),
               hasName("__libcpp_snprintf_l"),
               hasName("__libcpp_asprintf_l"),
               hasName("__libcpp_sscanf_l"),
               hasName("__tree_sub_invariant"),
               hasName("__stable_sort_move"),
               hasName("__stable_sort"),
               hasName("__stable_partition"),
               hasName("__lock_first"),
               hasName("__stable_partition_impl"),
               hasAttr(clang::attr::Visibility),
               hasAttr(clang::attr::AbiTag),
               cxxMethodDecl(), // We have explicitly instantiated classes and some of their methods don't have these attributes
               isDeleted(),
-              isConsteval())),
+              isConsteval(),
+              isInHideFromABINamespace())),
           isDefinition())
           .bind("missing_hide_from_abi"),
       this);
+
+  finder->addMatcher(cxxRecordDecl(isInHideFromABINamespace()).bind("struct_in_hide_from_abi_namespace"), this);
+
+  finder->addMatcher(functionDecl(isInHideFromABINamespace(), hasAttr(clang::attr::AbiTag))
+                         .bind("function_marked_hide_from_abi_in_hide_from_abi_namespace"),
+                     this);
 }
 
 void hide_from_abi::check(const clang::ast_matchers::MatchFinder::MatchResult& result) {
   if (const auto* call = result.Nodes.getNodeAs<clang::FunctionDecl>("missing_hide_from_abi"); call != nullptr) {
     diag(call->getBeginLoc(), "_LIBCPP_HIDE_FROM_ABI is missing");
+  } else if (const auto* record = result.Nodes.getNodeAs<clang::CXXRecordDecl>("struct_in_hide_from_abi_namespace");
+             record != nullptr) {
+    diag(record->getBeginLoc(), "structs should not be inside _LIBCPP_BEGIN_HIDE_FROM_ABI_NAMESPACE_STD");
+  } else if (const auto* func = result.Nodes.getNodeAs<clang::FunctionDecl>(
+                 "function_marked_hide_from_abi_in_hide_from_abi_namespace")) {
+    diag(func->getBeginLoc(),
+         "functions inside the hide-from-abi namespace should not be marked _LIBCPP_HIDE_FROM_ABI");
   }
 }
 } // namespace libcpp