[C++20][Modules] Implement P2615R1 revised export diagnostics.

It has been reported to that the current clang errors for, specifically, static_assert in export contexts are a serious blocker to adoption of modules in some cases. There is also implementation divergence with GCC and MSVC allowing the constructs mentioned below where clang currently rejects them with an error. The category of errors [for declarations in an exported context] is: (unnamed, static_assert, empty and asm decls). These are now permitted after P2615R1 which was approved by WG21 as a DR (and thus should be applied to C++20 as well). This patch removes these diagnostics and amends the testsuite accordingly. Differential Revision: https://reviews.llvm.org/D152946
llvm · Jun 24, 2023 · e5c7904 · e5c7904
1 parent 339a1f3
commit e5c7904
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 127 deletions.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11262,22 +11262,11 @@ def note_global_module_introducer_missing : Note<
 def err_export_within_anonymous_namespace : Error<
   "export declaration appears within anonymous namespace">;
 def note_anonymous_namespace : Note<"anonymous namespace begins here">;
-def ext_export_no_name_block : ExtWarn<
-  "ISO C++20 does not permit %select{an empty|a static_assert}0 declaration "
-  "to appear in an export block">, InGroup<ExportUnnamed>;
-def ext_export_no_names : ExtWarn<
-  "ISO C++20 does not permit a declaration that does not introduce any names "
-  "to be exported">, InGroup<ExportUnnamed>;
-def introduces_no_names : Error<
-  "declaration does not introduce any names to be exported">;
 def note_export : Note<"export block begins here">;
-def err_export_no_name : Error<
-  "%select{empty|static_assert|asm}0 declaration cannot be exported">;
-def ext_export_using_directive : ExtWarn<
-  "ISO C++20 does not permit using directive to be exported">,
-  InGroup<DiagGroup<"export-using-directive">>;
 def err_export_within_export : Error<
   "export declaration appears within another export declaration">;
+def err_export_anon_ns_internal : Error<
+  "anonymous namespaces cannot be exported">;
 def err_export_internal : Error<
   "declaration of %0 with internal linkage cannot be exported">;
 def err_export_using_internal : Error<

diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
@@ -814,76 +814,22 @@ Decl *Sema::ActOnStartExportDecl(Scope *S, SourceLocation ExportLoc,
   return D;
 }
 
-static bool checkExportedDeclContext(Sema &S, DeclContext *DC,
-                                     SourceLocation BlockStart);
-
-namespace {
-enum class UnnamedDeclKind {
-  Empty,
-  StaticAssert,
-  Asm,
-  UsingDirective,
-  Namespace,
-  Context
-};
-}
-
-static std::optional<UnnamedDeclKind> getUnnamedDeclKind(Decl *D) {
-  if (isa<EmptyDecl>(D))
-    return UnnamedDeclKind::Empty;
-  if (isa<StaticAssertDecl>(D))
-    return UnnamedDeclKind::StaticAssert;
-  if (isa<FileScopeAsmDecl>(D))
-    return UnnamedDeclKind::Asm;
-  if (isa<UsingDirectiveDecl>(D))
-    return UnnamedDeclKind::UsingDirective;
-  // Everything else either introduces one or more names or is ill-formed.
-  return std::nullopt;
-}
-
-unsigned getUnnamedDeclDiag(UnnamedDeclKind UDK, bool InBlock) {
-  switch (UDK) {
-  case UnnamedDeclKind::Empty:
-  case UnnamedDeclKind::StaticAssert:
-    // Allow empty-declarations and static_asserts in an export block as an
-    // extension.
-    return InBlock ? diag::ext_export_no_name_block : diag::err_export_no_name;
-
-  case UnnamedDeclKind::UsingDirective:
-    // Allow exporting using-directives as an extension.
-    return diag::ext_export_using_directive;
-
-  case UnnamedDeclKind::Namespace:
-    // Anonymous namespace with no content.
-    return diag::introduces_no_names;
-
-  case UnnamedDeclKind::Context:
-    // Allow exporting DeclContexts that transitively contain no declarations
-    // as an extension.
-    return diag::ext_export_no_names;
-
-  case UnnamedDeclKind::Asm:
-    return diag::err_export_no_name;
-  }
-  llvm_unreachable("unknown kind");
-}
+static bool checkExportedDecl(Sema &, Decl *, SourceLocation);
 
-static void diagExportedUnnamedDecl(Sema &S, UnnamedDeclKind UDK, Decl *D,
-                                    SourceLocation BlockStart) {
-  S.Diag(D->getLocation(), getUnnamedDeclDiag(UDK, BlockStart.isValid()))
-      << (unsigned)UDK;
-  if (BlockStart.isValid())
-    S.Diag(BlockStart, diag::note_export);
+/// Check that it's valid to export all the declarations in \p DC.
+static bool checkExportedDeclContext(Sema &S, DeclContext *DC,
+                                     SourceLocation BlockStart) {
+  bool AllUnnamed = true;
+  for (auto *D : DC->decls())
+    AllUnnamed &= checkExportedDecl(S, D, BlockStart);
+  return AllUnnamed;
 }
 
 /// Check that it's valid to export \p D.
 static bool checkExportedDecl(Sema &S, Decl *D, SourceLocation BlockStart) {
-  // C++2a [module.interface]p3:
-  //   An exported declaration shall declare at least one name
-  if (auto UDK = getUnnamedDeclKind(D))
-    diagExportedUnnamedDecl(S, *UDK, D, BlockStart);
 
-  //   [...] shall not declare a name with internal linkage.
+  //  C++20 [module.interface]p3:
+  //   [...] it shall not declare a name with internal linkage.
   bool HasName = false;
   if (auto *ND = dyn_cast<NamedDecl>(D)) {
     // Don't diagnose anonymous union objects; we'll diagnose their members
@@ -893,6 +839,7 @@ static bool checkExportedDecl(Sema &S, Decl *D, SourceLocation BlockStart) {
       S.Diag(ND->getLocation(), diag::err_export_internal) << ND;
       if (BlockStart.isValid())
         S.Diag(BlockStart, diag::note_export);
+      return false;
     }
   }
 
@@ -908,31 +855,29 @@ static bool checkExportedDecl(Sema &S, Decl *D, SourceLocation BlockStart) {
       S.Diag(Target->getLocation(), diag::note_using_decl_target);
       if (BlockStart.isValid())
         S.Diag(BlockStart, diag::note_export);
+      return false;
     }
   }
 
   // Recurse into namespace-scope DeclContexts. (Only namespace-scope
-  // declarations are exported.).
+  // declarations are exported).
   if (auto *DC = dyn_cast<DeclContext>(D)) {
-    if (isa<NamespaceDecl>(D) && DC->decls().empty()) {
-      if (!HasName)
-        // We don't allow an empty anonymous namespace (we don't allow decls
-        // in them either, but that's handled in the recursion).
-        diagExportedUnnamedDecl(S, UnnamedDeclKind::Namespace, D, BlockStart);
-      // We allow an empty named namespace decl.
-    } else if (DC->getRedeclContext()->isFileContext() && !isa<EnumDecl>(D))
-      return checkExportedDeclContext(S, DC, BlockStart);
+    if (!isa<NamespaceDecl>(D))
+      return true;
+
+    if (auto *ND = dyn_cast<NamedDecl>(D)) {
+      if (!ND->getDeclName()) {
+        S.Diag(ND->getLocation(), diag::err_export_anon_ns_internal);
+        if (BlockStart.isValid())
+          S.Diag(BlockStart, diag::note_export);
+        return false;
+      } else if (!DC->decls().empty() &&
+                 DC->getRedeclContext()->isFileContext()) {
+        return checkExportedDeclContext(S, DC, BlockStart);
+      }
+    }
   }
-  return false;
-}
-
-/// Check that it's valid to export all the declarations in \p DC.
-static bool checkExportedDeclContext(Sema &S, DeclContext *DC,
-                                     SourceLocation BlockStart) {
-  bool AllUnnamed = true;
-  for (auto *D : DC->decls())
-    AllUnnamed &= checkExportedDecl(S, D, BlockStart);
-  return AllUnnamed;
+  return true;
 }
 
 /// Complete the definition of an export declaration.
@@ -947,12 +892,7 @@ Decl *Sema::ActOnFinishExportDecl(Scope *S, Decl *D, SourceLocation RBraceLoc) {
     SourceLocation BlockStart =
         ED->hasBraces() ? ED->getBeginLoc() : SourceLocation();
     for (auto *Child : ED->decls()) {
-      if (checkExportedDecl(*this, Child, BlockStart)) {
-        // If a top-level child is a linkage-spec declaration, it might contain
-        // no declarations (transitively), in which case it's ill-formed.
-        diagExportedUnnamedDecl(*this, UnnamedDeclKind::Context, Child,
-                                BlockStart);
-      }
+      checkExportedDecl(*this, Child, BlockStart);
       if (auto *FD = dyn_cast<FunctionDecl>(Child)) {
         // [dcl.inline]/7
         // If an inline function or variable that is attached to a named module

diff --git a/clang/test/CXX/module/module.interface/p3.cpp b/clang/test/CXX/module/module.interface/p3.cpp
@@ -1,18 +1,19 @@
-// RUN: %clang_cc1 -std=c++2a %s -verify -pedantic-errors
+// RUN: %clang_cc1 -std=c++20 %s -verify -pedantic-errors
 
+// As amended by P2615R1 applied as a DR against C++20.
 export module p3;
 
 namespace A { int ns_mem; } // expected-note 2{{target}}
 
 // An exported declaration shall declare at least one name.
-export; // expected-error {{empty declaration cannot be exported}}
-export static_assert(true); // expected-error {{static_assert declaration cannot be exported}}
-export using namespace A;   // expected-error {{ISO C++20 does not permit using directive to be exported}}
+export; // No diagnostic after P2615R1 DR
+export static_assert(true); // No diagnostic after P2615R1 DR
+export using namespace A;   // No diagnostic after P2615R1 DR
 
-export { // expected-note 3{{export block begins here}}
-  ; // expected-error {{ISO C++20 does not permit an empty declaration to appear in an export block}}
-  static_assert(true); // expected-error {{ISO C++20 does not permit a static_assert declaration to appear in an export block}}
-  using namespace A;   // expected-error {{ISO C++20 does not permit using directive to be exported}}
+export { // No diagnostic after P2615R1 DR
+  ; // No diagnostic after P2615R1 DR
+  static_assert(true); // No diagnostic after P2615R1 DR
+  using namespace A;   // No diagnostic after P2615R1 DR
 }
 
 export struct {}; // expected-error {{must be class member}} expected-error {{GNU extension}} expected-error {{does not declare anything}}
@@ -24,27 +25,28 @@ export enum {} enum_;
 export enum E : int;
 export typedef int; // expected-error {{typedef requires a name}}
 export static union {}; // expected-error {{does not declare anything}}
-export asm(""); // expected-error {{asm declaration cannot be exported}}
+export asm(""); // No diagnostic after P2615R1 DR
 export namespace B = A;
 export using A::ns_mem; // expected-error {{using declaration referring to 'ns_mem' with module linkage cannot be exported}}
 namespace A {
   export using A::ns_mem; // expected-error {{using declaration referring to 'ns_mem' with module linkage cannot be exported}}
 }
 export using Int = int;
-export extern "C++" {} // expected-error {{ISO C++20 does not permit a declaration that does not introduce any names to be exported}}
-export extern "C++" { extern "C" {} } // expected-error {{ISO C++20 does not permit a declaration that does not introduce any names to be exported}}
+export extern "C++" {} // No diagnostic after P2615R1 DR
+export extern "C++" { extern "C" {} } // No diagnostic after P2615R1 DR
 export extern "C++" { extern "C" int extern_c; }
-export { // expected-note {{export block}}
+export { // No diagnostic after P2615R1 DR
   extern "C++" int extern_cxx;
-  extern "C++" {} // expected-error {{ISO C++20 does not permit a declaration that does not introduce any names to be exported}}
+  extern "C++" {} // No diagnostic after P2615R1 DR
 }
-export [[]]; // FIXME (bad diagnostic text): expected-error {{empty declaration cannot be exported}}
-export [[example::attr]]; // FIXME: expected-error {{empty declaration cannot be exported}} expected-warning {{unknown attribute 'attr'}}
+export [[]]; // No diagnostic after P2615R1 DR
+export [[example::attr]]; // expected-warning {{unknown attribute 'attr'}}
 
 // [...] shall not declare a name with internal linkage
 export static int a; // expected-error {{declaration of 'a' with internal linkage cannot be exported}}
 export static int b(); // expected-error {{declaration of 'b' with internal linkage cannot be exported}}
-export namespace { int c; } // expected-error {{declaration of 'c' with internal linkage cannot be exported}}
+export namespace { } // expected-error {{anonymous namespaces cannot be exported}}
+export namespace { int c; } // expected-error {{anonymous namespaces cannot be exported}}
 namespace { // expected-note {{here}}
   export int d; // expected-error {{export declaration appears within anonymous namespace}}
 }

diff --git a/clang/test/Modules/cxx20-10-2-ex1.cpp b/clang/test/Modules/cxx20-10-2-ex1.cpp
@@ -17,9 +17,9 @@ module;
 // expected-error@std-10-2-ex1.h:* {{export declaration can only be used within a module purview}}
 
 export module M1;
-export namespace {} // expected-error {{declaration does not introduce any names to be exported}}
-export namespace {
-int a1; // expected-error {{declaration of 'a1' with internal linkage cannot be exported}}
+export namespace {} // expected-error {{anonymous namespaces cannot be exported}}
+export namespace { // expected-error {{anonymous namespaces cannot be exported}}
+int a1;
 }
 namespace {    // expected-note {{anonymous namespace begins here}}
 export int a2; // expected-error {{export declaration appears within anonymous namespace}}
@@ -28,4 +28,4 @@ export static int b; // expected-error {{declaration of 'b' with internal linkag
 export int f();      // OK
 
 export namespace N {}     // namespace N
-export using namespace N; // expected-error {{ISO C++20 does not permit using directive to be exported}}
+export using namespace N; // No diagnostic after P2615R1 DR
diff --git a/clang/test/Modules/cxx20-10-2-ex7.cpp b/clang/test/Modules/cxx20-10-2-ex7.cpp
@@ -2,8 +2,10 @@
 
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -verify -o %t
 
+// expected-no-diagnostics
+
 export module M;
 export namespace N {
 int x;                 // OK
-static_assert(1 == 1); // expected-error {{static_assert declaration cannot be exported}}
+static_assert(1 == 1); // No diagnostic after P2615R1 DR
 } // namespace N
diff --git a/clang/test/SemaCXX/modules.cppm b/clang/test/SemaCXX/modules.cppm
@@ -34,11 +34,11 @@ int use_a = a; // expected-error {{use of undeclared identifier 'a'}}
 import foo; // expected-error {{imports must immediately follow the module declaration}}
 
 export {}
-export {  // expected-note {{begins here}}
-  ;       // expected-warning {{ISO C++20 does not permit an empty declaration to appear in an export block}}
+export {
+  ;       // No diagnostic after P2615R1 DR
 }
-export {               // expected-note {{begins here}}
-  static_assert(true); // expected-warning {{ISO C++20 does not permit a static_assert declaration to appear in an export block}}
+export {
+  static_assert(true); // No diagnostic after P2615R1 DR
 }
 
 int use_b = b; // expected-error{{use of undeclared identifier 'b'}}