[clang] e5c7904 - [C++20][Modules] Implement P2615R1 revised export diagnostics.
Iain Sandoe via cfe-commits
cfe-commits at lists.llvm.org
Sat Jun 24 01:02:24 PDT 2023
Author: Iain Sandoe
Date: 2023-06-24T09:01:59+01:00
New Revision: e5c7904fa0bfa5a24f192cfa7b9116560e1f5d43
URL: https://github.com/llvm/llvm-project/commit/e5c7904fa0bfa5a24f192cfa7b9116560e1f5d43
DIFF: https://github.com/llvm/llvm-project/commit/e5c7904fa0bfa5a24f192cfa7b9116560e1f5d43.diff
LOG: [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
Added:
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaModule.cpp
clang/test/CXX/module/module.interface/p3.cpp
clang/test/Modules/cxx20-10-2-ex1.cpp
clang/test/Modules/cxx20-10-2-ex7.cpp
clang/test/SemaCXX/modules.cppm
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a215095d540ae..3f0b7a54d8890 100644
--- 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
index 5ce5330b09466..9b2982e7fa4df 100644
--- 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
index 2d888d2018e87..32819b2dccb11 100644
--- 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
index 007f080804836..0cd6f77466f4b 100644
--- 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 at 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
index 07029df876d29..e458df2d7f5c3 100644
--- 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
index 39d8275f38aad..409a6b2f1661e 100644
--- 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'}}
More information about the cfe-commits
mailing list