[libcxx-commits] [libcxx] [libc++][modules] Adds std.compat module. (PR #71438)

[Louis Dionne via libcxx-commits]

================
@@ -199,50 +223,121 @@ static bool is_viable_declaration(const clang::NamedDecl* decl) {
 /// Returns the name is a reserved name.
 ///
 /// Detected reserved names are names starting with __ or _[A-Z].
-/// These names can be in the namespace std or any namespace inside std. For
-/// example std::ranges contains reserved names to implement the Niebloids.
+/// 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 2 candidates which are not used in libc++
+/// This test misses candidates which are not used in libc++
 /// * any identifier with two underscores not at the start
-/// * a name with a leading underscore in the global namespace
-bool is_reserved_name(const std::string& name) {
+bool is_reserved_name(std::string_view name) {
+  if (name.starts_with("_"))
+    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_valid_global_name(std::string_view name) {
+  static const std::set<std::string_view> valid{
+      "operator delete", "operator delete[]", "operator new", "operator new[]"};
+  return valid.contains(name);
+}
+
+static bool is_valid_declaration_context(
+    const clang::NamedDecl& decl, std::string_view name, header_exportable_declarations::FileType file_type) {
+  if (decl.getDeclContext()->isNamespace())
+    return true;
+
+  if (is_valid_global_name(name))
+    return true;
+
+  return file_type != header_exportable_declarations::FileType::Header;
+}
+
+static bool is_module(header_exportable_declarations::FileType file_type) {
+  switch (file_type) {
+  case header_exportable_declarations::FileType::Module:
+  case header_exportable_declarations::FileType::ModulePartition:
+  case header_exportable_declarations::FileType::CompatModule:
+  case header_exportable_declarations::FileType::CompatModulePartition:
+    return true;
+
+  case header_exportable_declarations::FileType::Header:
+  case header_exportable_declarations::FileType::CHeader:
+    return false;
+
+  case header_exportable_declarations::FileType::Unknown:
+    llvm::errs() << "This should be unreachable.\n";
+    break;
+  }
+}
+
 void header_exportable_declarations::check(const clang::ast_matchers::MatchFinder::MatchResult& result) {
   if (const auto* decl = result.Nodes.getNodeAs<clang::NamedDecl>("header_exportable_declarations"); decl != nullptr) {
     if (!is_viable_declaration(decl))
       return;
 
     std::string name = get_qualified_name(*decl);
+    if (name.empty())
+      return;
+
     if (is_reserved_name(name))
       return;
 
     // For modules only take the declarations exported.
-    if (file_type_ == FileType::ModulePartition || file_type_ == FileType::Module)
+    if (is_module(file_type_))
       if (decl->getModuleOwnershipKind() != clang::Decl::ModuleOwnershipKind::VisibleWhenImported)
         return;
 
+    if (!is_valid_declaration_context(*decl, name, file_type_))
+      return;
+
     if (decls_.contains(name)) {
       // For modules avoid exporting the same named declaration twice. For
       // header files this is common and valid.
-      if (file_type_ == FileType::ModulePartition)
+      if (file_type_ == FileType::ModulePartition || file_type_ == FileType::CompatModulePartition)
         // After the warning the script continues.
         // The test will fail since modules have duplicated entries and headers not.
         llvm::errs() << "Duplicated export of '" << name << "'.\n";
       else
         return;
     }
 
-    std::cout << "using " << std::string{name} << ";\n";
+    // For named declarations in std this is valid
+    //   using std::foo;
+    // for named declarations it is invalid to use
+    //   using bar;
+    // Since fully qualifying named declarations in the std namespace is valid
+    // using fully qualified names unconditionally.
+    std::cout << "using ::" << std::string{name} << ";\n";
     decls_.insert(name);
+  } else if (const auto* decl = result.Nodes.getNodeAs<clang::NamedDecl>("cheader_exportable_declarations");
+             decl != nullptr) {
+    if (decl->getDeclContext()->isNamespace())
+      return;
+
+    if (!is_viable_declaration(decl))
+      return;
+
+    std::string name = get_qualified_name(*decl);
+    if (is_reserved_name(name))
----------------
ldionne wrote:

Do we see the declaration of `::_Exit` here? If so do we filter reserved names correctly? I think the result is that right now we're not generating a test for std.compat's definition of `::_Exit`.

https://github.com/llvm/llvm-project/pull/71438