[clang][sema] Allow unnamed decls in C++20 module export{} blocks

[tbaeder]

Summary

As part of P1766R1, allow unnamed decls in export{} blocks.

I've adjusted the module-ts tests as well, but I am (generally) not sure whether code for C++20 modules should affect the modules-ts cases. What's the policy here?

[tbaeder]

Reading this again, it sounds like this is not expected to happen for C++20; I was using https://clang.llvm.org/cxx_status.html#cxx20 as guidance here.

[tbaeder]

Limited the change to c++2b

[ChuanqiXu]

clang/lib/Sema/SemaModule.cpp

650–652

Should we add a cite to P1766R1 here?

clang/test/CXX/module/module.interface/p3.cpp

12–16 ↗

(On Diff #402813)

This one should be deleted unintentionally.

[tbaeder]

clang/lib/Sema/SemaModule.cpp

650–652

I wanted to do that, but I'm not 100% sure how. P1766R1 doesn't seem to change any wording for this change. What exactly do I cite?

clang/test/CXX/module/module.interface/p3.cpp

12–16 ↗

(On Diff #402813)

Right, I forgot about that. Added it back, thanks.

[ChuanqiXu]

clang/lib/Sema/SemaModule.cpp

650–652

I am not sure about the standard method. But I guess it might not be bad to write something like:

// P1766R1 removed the rule that a declaration in an export block introduces at least one name. The rule for the single-declaration form of export is retained.
// FIXME: Cite for labels when it shows in the draft.

BTW, you could send a pull request or an issue at: https://github.com/cplusplus/draft to do the change. Since P1766R1 is adopted, so this change should be editorial.

[tbaeder]

clang/lib/Sema/SemaModule.cpp

650–652

Thanks, I added a comment and opened https://github.com/cplusplus/draft/issues/5237

[ChuanqiXu]

Oh, it looks like we couldn't go on since the part of PR1766R1 is not adopted: https://github.com/cplusplus/draft/issues/5237

clang/lib/Sema/SemaModule.cpp

static llvm::Optional<UnnamedDeclKind> getUnnamedDeclKind(Decl *D) {
   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 llvm::None;
 }
 
-unsigned getUnnamedDeclDiag(UnnamedDeclKind UDK, bool InBlock) {
+static 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::Context:
     // Allow exporting DeclContexts that transitively contain no declarations
static void diagExportedUnnamedDecl(Sema &S, UnnamedDeclKind UDK, Decl *D,
   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 \p D.
 static bool checkExportedDecl(Sema &S, Decl *D, SourceLocation BlockStart) {
+  if (auto UDK = getUnnamedDeclKind(D)) {
+    if (S.getLangOpts().CPlusPlus2b) {
+      // P1766R1 allows unnamed declarations in export{} blocks, but not in
+      // the single-declaration form of export.
+      // FIXME: Cite the standard once P1766R1 is reflected in the wording
+      if (BlockStart.isInvalid())
+        diagExportedUnnamedDecl(S, *UDK, D, BlockStart);
+    } else {
       // 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.
   if (auto *ND = dyn_cast<NamedDecl>(D)) {
     // Don't diagnose anonymous union objects; we'll diagnose their members
     // instead.
     if (ND->getDeclName() && ND->getFormalLinkage() == InternalLinkage) {
       S.Diag(ND->getLocation(), diag::err_export_internal) << ND;
       if (BlockStart.isValid())

clang/test/Modules/cxx20-export.cpp

+
+// RUN: %clang_cc1 -std=c++2b -verify %s
+export module testmodule;
+
+export static_assert(true); // expected-error {{static_assert declaration cannot be exported}}
+export;                     // expected-error {{empty declaration cannot be exported}}
+
+namespace A {};
+
+export {
+#define STR(x) constexpr char x[] = #x;
+  STR(foo);
+#undef STR
+
+  static_assert(true);
+
+  template <typename T> struct X { T t; };
+  template <typename T> X(T) -> X<T>;
+
+  ; // Empty declaration
+
+  using namespace A;
+}