[libc++][module] Fixes std::string UDL.

[mordante]

The fix changes the way the validation script determines the qualified name of a declaration. Inline namespaces without a reserved name are now always part of the name. The Clang code only does this when the names are ambigious. This method is often used for the operator""foo for UDLs.

Adjusted the newly flagged issue and removed a work-around in the test code that is no longer required.

Fixes #72427

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

The fix changes the way the validation script determines the qualified name of a declaration. Inline namespaces without a reserved name are now always part of the name. The Clang code only does this when the names are ambigious. This method is often used for the operator""foo for UDLs.

Adjusted the newly flagged issue and removed a work-around in the test code that is no longer required.

Fixes #72427

---

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

3 Files Affected:

• (modified) libcxx/modules/std/string.inc (-5)
• (modified) libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp (+32-44)
• (modified) libcxx/utils/libcxx/test/modules.py (-8)

diff --git a/libcxx/modules/std/string.inc b/libcxx/modules/std/string.inc
index 8366690fd9d37..2b98ec5cb732b 100644
--- a/libcxx/modules/std/string.inc
+++ b/libcxx/modules/std/string.inc
@@ -67,15 +67,10 @@ export namespace std {
   // [basic.string.hash], hash support
   using std::hash;
 
-  // TODO MODULES is this a bug?
-#if _LIBCPP_STD_VER >= 23
-  using std::operator""s;
-#else
   inline namespace literals {
     inline namespace string_literals {
       // [basic.string.literals], suffix for basic_string literals
       using std::literals::string_literals::operator""s;
     } // namespace string_literals
   }   // namespace literals
-#endif
 } // namespace std
diff --git a/libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp b/libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp
index fcb5865adf0d4..d39e13be8539d 100644
--- a/libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp
+++ b/libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp
@@ -166,7 +166,7 @@ void header_exportable_declarations::registerMatchers(clang::ast_matchers::Match
   }
 }
 
-/// Returns the qualified name of a declaration.
+/// Returns the qualified name of a public declaration.
 ///
 /// There is a small issue with qualified names. Typically the name returned is
 /// in the namespace \c std instead of the namespace \c std::__1. Except when a
@@ -182,14 +182,37 @@ void header_exportable_declarations::registerMatchers(clang::ast_matchers::Match
 /// * exception has equality operators for the type \c exception_ptr
 /// * initializer_list has the functions \c begin and \c end
 ///
-/// \warning In some cases the returned name can be an empty string.
-/// The cause has not been investigated.
+/// When the named declaration uses a reserved name the result is an
+/// empty string.
 static std::string get_qualified_name(const clang::NamedDecl& decl) {
-  std::string result = decl.getQualifiedNameAsString();
-
-  if (result.starts_with("std::__1::"))
-    result.erase(5, 5);
-
+  std::string result = decl.getNameAsString();
+  // Reject reserved names (ignoring _ in global namespace).
+  if (result.size() >= 2 && result[0] == '_')
+    if (result[1] == '_' || std::isupper(result[1]))
+      if (result != "_Exit")
+        return "";
+
+  for (auto* context = llvm::dyn_cast_or_null<clang::NamespaceDecl>(decl.getDeclContext()); //
+       context;
+       context = llvm::dyn_cast_or_null<clang::NamespaceDecl>(context->getDeclContext())) {
+    std::string ns = std::string(context->getName());
+
+    if (ns.starts_with("__")) {
+      // When the reserved name is an inline namespace the namespace is
+      // not added to the qualified name instead of removed. Libc++ uses
+      // several inline namespace with reserved names. For example,
+      // __1 for every declaration, __cpo in range-based algorithms.
+	  //
+	  // Note other inline namespaces are expanded. This resolves
+	  // ambiguity when two named declarations have the same name but in
+	  // different inline namespaces. These typically are the literal
+	  // conversion operators like operator""s which can be a
+	  // std::string or std::chrono::seconds.
+      if (!context->isInline())
+        return "";
+    } else
+      result = ns + "::" + result;
+  }
   return result;
 }
 
@@ -220,38 +243,6 @@ static bool is_viable_declaration(const clang::NamedDecl* decl) {
   return llvm::isa<clang::EnumDecl, clang::VarDecl, clang::ConceptDecl, clang::TypedefNameDecl, clang::UsingDecl>(decl);
 }
 
-/// Returns the name is a reserved name.
-///
-/// Detected reserved names are names starting with __ or _[A-Z].
-/// These names can be in the global namespace, std namespace or any namespace
-/// inside std. For example, std::ranges contains reserved names to implement
-/// the Niebloids.
-///
-/// This test misses candidates which are not used in libc++
-/// * any identifier with two underscores not at the start
-bool is_reserved_name(std::string_view name) {
-  if (name.starts_with("_")) {
-    // This is a public name declared in cstdlib.
-    if (name == "_Exit")
-      return false;
-
-    return name.size() > 1 && (name[1] == '_' || std::isupper(name[1]));
-  }
-
-  std::size_t pos = name.find("::_");
-  if (pos == std::string::npos)
-    return false;
-
-  if (pos + 3 > name.size())
-    return false;
-
-  // This is a public name declared in cstdlib.
-  if (name == "std::_Exit")
-    return false;
-
-  return name[pos + 3] == '_' || std::isupper(name[pos + 3]);
-}
-
 /// Some declarations in the global namespace are exported from the std module.
 static bool is_global_name_exported_by_std_module(std::string_view name) {
   static const std::set<std::string_view> valid{
@@ -297,9 +288,6 @@ void header_exportable_declarations::check(const clang::ast_matchers::MatchFinde
     if (name.empty())
       return;
 
-    if (is_reserved_name(name))
-      return;
-
     // For modules only take the declarations exported.
     if (is_module(file_type_))
       if (decl->getModuleOwnershipKind() != clang::Decl::ModuleOwnershipKind::VisibleWhenImported)
@@ -336,7 +324,7 @@ void header_exportable_declarations::check(const clang::ast_matchers::MatchFinde
       return;
 
     std::string name = get_qualified_name(*decl);
-    if (is_reserved_name(name))
+    if (name.empty())
       return;
 
     if (global_decls_.contains(name))
diff --git a/libcxx/utils/libcxx/test/modules.py b/libcxx/utils/libcxx/test/modules.py
index deaac450381c3..bd19fac314dd9 100644
--- a/libcxx/utils/libcxx/test/modules.py
+++ b/libcxx/utils/libcxx/test/modules.py
@@ -141,14 +141,6 @@ def process_module_partition(self, header, is_c_header):
                 f"#  include <{header}>{nl}"
                 f"#endif{nl}"
             )
-        elif header == "chrono":
-            # When localization is disabled the header string is not included.
-            # When string is included chrono's operator""s is a named declaration
-            #   using std::chrono_literals::operator""s;
-            # else it is a named declaration
-            #   using std::operator""s;
-            # TODO MODULES investigate why
-            include = f"#include <string>{nl}#include <chrono>{nl}"
         else:
             include = f"#include <{header}>{nl}"
 

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 42e4967140e345923a43f809ba69be57200f46ae 9b005e3d3dc4e4b0f4781b8d8c9b0a2803cc2a60 -- libcxx/modules/std/string.inc libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp

View the diff from clang-format here.

diff --git a/libcxx/modules/std/string.inc b/libcxx/modules/std/string.inc
index c83ee7643f..2b98ec5cb7 100644
--- a/libcxx/modules/std/string.inc
+++ b/libcxx/modules/std/string.inc
@@ -72,5 +72,5 @@ export namespace std {
       // [basic.string.literals], suffix for basic_string literals
       using std::literals::string_literals::operator""s;
     } // namespace string_literals
-  } // namespace literals
+  }   // namespace literals
 } // namespace std

[ldionne]

LGTM with tabs fixed!