[clang][Modules] Complete the implementation of P2615: Meaningful exports

clang/docs/ReleaseNotes.rst

@@ -49,6 +49,10 @@ C++ Specific Potentially Breaking Changes
 - Clang now correctly rejects ``export`` declarations in module implementation
   partitions. (#GH107602)
 
+- Clang now correctly rejects explicit instantiations and specializations
+  marked with ``export`` or a language linkage specification, completing
+  its implementation of P2615. (#GH160016)
+
 ABI Changes in This Version
 ---------------------------
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -12854,6 +12854,15 @@ def err_invalid_module_name : Error<"%0 is an invalid name for a module">;
 def err_extern_def_in_header_unit : Error<
   "non-inline external definitions are not permitted in C++ header units">;
 
+def err_meaningless_export : Error<
+  "%select{an explicit instantiation|a specialization}0 cannot be "
+  "marked 'export'">;
+def note_meaningless_export_explanation : Note<
+  "as long as its primary template is exported, it will be too">;
+def err_invalid_decl_in_linkage_spec : Error<
+  "language linkage specification cannot be applied to "
+  "%select{an explicit instantiation|a specialization|an export declaration}0">;
+
 def warn_exposure : Warning <
   "TU local entity %0 is exposed">,
   InGroup<DiagGroup<"TU-local-entity-exposure">>;

clang/include/clang/Parse/Parser.h

@@ -3192,7 +3192,7 @@ class Parser : public CodeCompletionHandler {
   /// \verbatim
   ///       linkage-specification: [C++ 7.5p2: dcl.link]
   ///         'extern' string-literal '{' declaration-seq[opt] '}'
-  ///         'extern' string-literal declaration
+  ///         'extern' string-literal name-declaration
   /// \endverbatim
   ///
   Decl *ParseLinkage(ParsingDeclSpec &DS, DeclaratorContext Context);
@@ -3201,7 +3201,7 @@ class Parser : public CodeCompletionHandler {
   ///
   /// \verbatim
   ///       export-declaration:
-  ///         'export' declaration
+  ///         'export' name-declaration
   ///         'export' '{' declaration-seq[opt] '}'
   /// \endverbatim
   ///
@@ -3217,6 +3217,29 @@ class Parser : public CodeCompletionHandler {
   ///
   Decl *ParseExportDeclaration();
 
+  /// Ensure the declaration in an unbraced linkage-specification or
+  /// export-declaration is not an explicit-instantiation,
+  /// explicit-specialization, or export-declaration:
+  ///
+  /// \verbatim
+  ///       export-declaration: [C++: module.interface]
+  ///         export name-declaration
+  ///
+  ///       linkage-specification: [C++: dcl.link]
+  ///         export name-declaration
+  ///
+  ///       declaration: [C++: dcl.pre]
+  ///         name-declaration
+  ///         special-declaration
+  ///
+  ///       special-declaration: [C++: dcl.pre]
+  ///         explicit-instantiation
+  ///         explicit-specialization
+  ///         export-declaration
+  /// \endverbatim
+  ///
+  void CheckUnbracedLinkageOrExportDeclaration(Decl *LinkageOrExportDecl);
+
   /// ParseUsingDirectiveOrDeclaration - Parse C++ using using-declaration or
   /// using-directive. Assumes that current token is 'using'.
   DeclGroupPtrTy ParseUsingDirectiveOrDeclaration(

clang/lib/Parse/ParseDeclCXX.cpp

@@ -339,9 +339,11 @@ Decl *Parser::ParseLinkage(ParsingDeclSpec &DS, DeclaratorContext Context) {
     // ... but anyway remember that such an "extern" was seen.
     DS.setExternInLinkageSpec(true);
     ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs, &DS);
-    return LinkageSpec ? Actions.ActOnFinishLinkageSpecification(
-                             getCurScope(), LinkageSpec, SourceLocation())
-                       : nullptr;
+    if (!LinkageSpec)
+      return nullptr;
+    CheckUnbracedLinkageOrExportDeclaration(LinkageSpec);
+    return Actions.ActOnFinishLinkageSpecification(getCurScope(), LinkageSpec,
+                                                   SourceLocation());
   }
 
   DS.abort();
@@ -420,6 +422,7 @@ Decl *Parser::ParseExportDeclaration() {
     MaybeParseCXX11Attributes(DeclAttrs);
     ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
     ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs);
+    CheckUnbracedLinkageOrExportDeclaration(ExportDecl);
     return Actions.ActOnFinishExportDecl(getCurScope(), ExportDecl,
                                          SourceLocation());
   }
@@ -440,6 +443,52 @@ Decl *Parser::ParseExportDeclaration() {
                                        T.getCloseLocation());
 }
 
+void Parser::CheckUnbracedLinkageOrExportDeclaration(
+    Decl *LinkageOrExportDecl) {
+  const auto *DC = cast<DeclContext>(LinkageOrExportDecl);
+  if (DC->decls_empty())
+    return;
+
+  const Decl *D = *DC->decls_begin();
+
+  // Nested export declarations are diagnosed elsewhere.
+  if (isa<LinkageSpecDecl>(LinkageOrExportDecl) && isa<ExportDecl>(D)) {
+    Diag(LinkageOrExportDecl->getLocation(),
+         diag::err_invalid_decl_in_linkage_spec)
+        << 2;
+    return;
+  }
+
+  TemplateSpecializationKind TSK = [&] {
+    if (const auto *EID = dyn_cast<ExplicitInstantiationDecl>(D))
+      return EID->getTemplateSpecializationKind();
+    if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D))
+      return CTSD->getTemplateSpecializationKind();
+    if (const auto *VTSD = dyn_cast<VarTemplateSpecializationDecl>(D))
+      return VTSD->getTemplateSpecializationKind();
+    if (const auto *FD = dyn_cast<FunctionDecl>(D))
+      return FD->getTemplateSpecializationKind();
+    return TSK_Undeclared;
+  }();
+
+  if (TSK == TSK_Undeclared)
+    return;
+
+  if (const auto *ED = dyn_cast<ExportDecl>(LinkageOrExportDecl)) {
+    Diag(ED->getExportLoc(), diag::err_meaningless_export)
+        << (TSK == TSK_ExplicitSpecialization)
+        << FixItHint::CreateRemoval(ED->getExportLoc());
+    Diag(ED->getExportLoc(), diag::note_meaningless_export_explanation);
+    return;
+  }
+
+  if (const auto *LS = dyn_cast<LinkageSpecDecl>(LinkageOrExportDecl)) {
+    Diag(LS->getLocation(), diag::err_invalid_decl_in_linkage_spec)
+        << (TSK == TSK_ExplicitSpecialization);
+    return;
+  }
+}
+
 Parser::DeclGroupPtrTy Parser::ParseUsingDirectiveOrDeclaration(
     DeclaratorContext Context, const ParsedTemplateInfo &TemplateInfo,
     SourceLocation &DeclEnd, ParsedAttributes &Attrs) {

clang/test/Modules/explicit-specializations.cppm

@@ -20,7 +20,7 @@ struct S {
 
 export struct A {};
 
-export template <>
+template <>
 struct S<A> {
     static constexpr bool selected = true;
 };
@@ -34,7 +34,7 @@ struct V {
     static constexpr bool selected = false;
 };
 
-export template <>
+template<>
 struct V<S> {
     static constexpr bool selected = true;
 };
@@ -46,7 +46,7 @@ struct Numbers {
     static constexpr int value = X;
 };
 
-export template<>
+template<>
 struct Numbers<43> {
     static constexpr bool selected = true;
     static constexpr int value = 43;
@@ -58,7 +58,7 @@ struct Pointers {
 };
 
 export int IntegralValue = 0;
-export template<>
+template<>
 struct Pointers<&IntegralValue> {
     static constexpr bool selected = true;
 };
@@ -68,7 +68,7 @@ struct NullPointers {
     static constexpr bool selected = false;
 };
 
-export template<>
+template<>
 struct NullPointers<nullptr> {
     static constexpr bool selected = true;
 };
@@ -79,7 +79,7 @@ struct Array {
 };
 
 export int array[5];
-export template<>
+template<>
 struct Array<array> {
     static constexpr bool selected = true;
 };

clang/test/Modules/export-language-linkage.cppm

@@ -55,7 +55,7 @@ extern "C++" {
     int h();
 }
 
-extern "C++" export int g();
+export int g();
 
 //--- d.cpp
 import c;

clang/test/Modules/merge-var-template-spec-cxx-modules.cppm

@@ -28,7 +28,7 @@ template <class T> constexpr T* zero<T*> = nullptr; // expected-error-re {{decla
                                                     // expected-note@* {{previous}}
 
 template <> constexpr int** zero<int**> = nullptr; // ok, new specialization.
-template <class T> constexpr T** zero<T**> = nullptr; // ok, new partial specilization.
+template <class T> constexpr T** zero<T**> = nullptr; // ok, new partial specialization.
 
 //--- var_def.cppm
 export module var_def;
@@ -37,8 +37,16 @@ export template <class T> constexpr T zero = 0;
 export struct Int {
     int value;
 };
-export template <> constexpr Int zero<Int> = {0};
-export template <class T> constexpr T* zero<T*> = nullptr;
+
+// FIXME: it should make no difference whether a specialization is
+// exported or not, but currently, removing this 'export'
+// leads to an assertion failure in Sema::shouldLinkPossiblyHiddenDecl.
+export {
+
+template <> constexpr Int zero<Int> = {0};
+template <class T> constexpr T* zero<T*> = nullptr;
+
+}
 
 //--- reexport1.cppm
 export module reexport1;

clang/test/Modules/pr59780.cppm

@@ -25,7 +25,7 @@ export module a;
 export template<typename T>
 int x = 0;
 
-export template<>
+template<>
 int x<int> = 0;
 
 export template<typename T>
@@ -36,7 +36,7 @@ struct Y {
 template <typename T>
 int Y<T>::value = 0;
 
-export template<>
+template<>
 struct Y<int> {
     static int value;
 };

clang/test/Modules/pr60890.cppm

@@ -24,12 +24,12 @@ struct a {
 	void aaa() requires(true) {}
 };
 
-export template struct a<double>;
+template struct a<double>;
 
 export template<typename T>
 void foo(T) requires(true) {}
 
-export template void foo<double>(double);
+template void foo<double>(double);
 
 export template <typename T>
 class A {

clang/test/Modules/pr97313.cppm

@@ -76,19 +76,19 @@ public:
     virtual ~Template();
 };
 
-export template<>
+template<>
 class Template<char> {
 public:
     virtual ~Template();
 };
 
 // CHECK: @_ZTIW3Mod8TemplateIcE = {{.*}}constant
 
-export template class Template<unsigned>;
+template class Template<unsigned>;
 
 // CHECK: @_ZTIW3Mod8TemplateIjE = {{.*}}weak_odr
 
-export extern template class Template<double>;
+extern template class Template<double>;
 
 auto v = new Template<signed int>();
 

clang/test/Modules/template-function-specialization.cpp

@@ -42,7 +42,7 @@ export template <typename T>
 void foo4() {
 }
 
-export template <>
+template <>
 void foo4<int>() {
 }