[C++20][Modules] Implement P2615R1 exported specialization diagnostics.

[iains]

Summary

P2615R1 introduces diagnostics for partial and explicit specializations in module export declarations. This is applied as a DR to C++20.

Two testcases are removed since they were testing behaviour of imported specialisations (which are no longer permitted as exports).
Three testcases are amended to remove exports of specialisations, but retain the remainder of the test.

[iains]

I think it is a good idea to keep this part of P2615R1 separate from the unnamed exports diagnostics [the parent commit] - it seems likely that there will be more fallout from this (given that there were already PRs about the behaviour of exported specialisations).

[ChuanqiXu]

clang/include/clang/Basic/DiagnosticSemaKinds.td

11275–11278

According to #[dcl.pre] in https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2615r1.html, the export for explicit instantiation is also allowed.

clang/lib/Sema/SemaModule.cpp

831–832

nit

834

nit

836

845–846

847–848

nit

848–849

Given P2615R1 doesn't allow explicit-instantiation in export block too.

851

ditto

clang/test/CXX/temp/temp.explicit/p2-p2615r1.cpp

20

Let's add another case for explicit instantiation.

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

35–36

It should be good enough to remove the "export" in the template specialization.

clang/test/Modules/pr59780.cppm

18–19

It should be good to remove the "export" in the template specialization.

clang/test/Modules/pr60890.cppm

21

The specialization is meaningful here to test the serializer/deserializer can handle the merge well. It is OK to remove the export here in this patch and I'll try to update the tests to make its semantics more clear.

clang/test/Modules/pr62796.cppm

42–43

In case it is not allowed to export the explicit instantiations, we should move it out of the export block instead of removing it.

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

42

ditto

clang/unittests/Serialization/VarDeclConstantInitTest.cpp

89

Same as above, in this case we should move it instead of removing it.

118

We don't need this.

[ChuanqiXu]

BTW, since this will break existing code. Let's mention this in ReleaseNotes.

[iains]

clang/lib/Sema/SemaModule.cpp

848–849

I think I must be missing something - I do not see that in the paper - please could you expand your comment?

[ChuanqiXu]

clang/lib/Sema/SemaModule.cpp

848–849

See the proposed change in #[module.interface]p1 in the paper, it changed:

export declaration

to

export named-declaration

And the change between the old declaration and the new named declaration is in the proposed change in #[dcl.pre]p1. So it shows the special declarations shouldn't be allowed after export.

Also I noted it is allowed to write export { declaration-seq_opt } where the declaration-seq may contain special declarations. It looks weird to me. I'll try to check it and ask WG21 when necessary.

[ChuanqiXu]

clang/lib/Sema/SemaModule.cpp

848–849

I checked the record. And it shows that the current wording reflects the intention. I mean

export template<> class A<int>;

is not allowed. But,

export { template<> class A<int> };

is allowed.

[iains]

need to re-check that the intention of the paper is covered (since we currently treat bare 'export' and 'export {}' in a similar manner).

clang/lib/Sema/SemaModule.cpp

848–849

See the proposed change in #[module.interface]p1 in the paper, it changed:

...

And the change between the old declaration and the new named declaration is in the proposed change in #[dcl.pre]p1. So it shows the special declarations shouldn't be allowed after export.

Also I noted it is allowed to write export { declaration-seq_opt } where the declaration-seq may contain special declarations. It looks weird to me. I'll try to check it and ask WG21 when necessary.

Indeed that does seem to be a strange mismatch - but you are right the introduction does mention this:

"This paper addresses CWG2443 by forbidding applying export directly to certain meaningless declarations. Conversely, it also allows all kinds to appear with export {}. For consistency extern "C" is given the same restrictions when used directly."

so I think I need to recheck this patch.

clang/include/clang/Basic/DiagnosticSemaKinds.td

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<
   "using declaration referring to %1 with %select{internal|module|unknown}0 "
   "linkage cannot be exported">;
+def err_export_partial_specialization : Error<
+  "partial %select{class|variable}0 specialization %1 cannot be exported">;
+def err_export_explicit_specialization : Error<
+  "explicit specialization %0 cannot be exported">;
 def err_export_not_in_module_interface : Error<
   "export declaration can only be used within a module purview">;
 def err_export_inline_not_defined : Error<
   "inline function not defined%select{| before the private module fragment}0">;
 def err_export_partition_impl : Error<
   "module partition implementations cannot be exported">;
 def err_export_in_private_module_fragment : Error<
   "export declaration cannot be used in a private module fragment">;

clang/lib/Sema/SemaModule.cpp

static bool checkExportedDeclContext(Sema &S, DeclContext *DC,
   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) {
 
+  if (isa<ClassTemplatePartialSpecializationDecl>(D) ||
+      isa<VarTemplatePartialSpecializationDecl>(D)) {
+    // C++20 [module.interface]p1:
+    //   [...] shall not declare a partial specialization.
+    int Kind = isa<ClassTemplatePartialSpecializationDecl>(D) ? 0 : 1;
+    auto *ND = dyn_cast<NamedDecl>(D);
+    S.Diag(ND->getLocation(), diag::err_export_partial_specialization)
+        << Kind << ND;
+    if (BlockStart.isValid())
+      S.Diag(BlockStart, diag::note_export);
+  } else if (auto *ND = dyn_cast<NamedDecl>(D)) {
+    // C++20 [temp.expl.spec]p2
+    //   The declaration in an explicit-specialization shall not be an
+    //   export-declaration.
+    bool BadExport = isa<ClassTemplateSpecializationDecl>(ND) ||
+                     isa<VarTemplateSpecializationDecl>(ND);
+    if (auto *FD = dyn_cast<FunctionDecl>(D)) {
+      if (FD->getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
+        BadExport = true;
+    } else if (auto *VD = dyn_cast<VarDecl>(D)) {
+      if (VD->getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
+        BadExport = true;
+    } else if (auto *ED = dyn_cast<EnumDecl>(D)) {
+      if (ED->getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
+        BadExport = true;
+    }
+    if (BadExport) {
+      S.Diag(ND->getLocation(), diag::err_export_explicit_specialization) << ND;
+      if (BlockStart.isValid())
+        S.Diag(BlockStart, diag::note_export);
+    }
+  }
+
   //  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
     // instead.
     HasName = (bool)ND->getDeclName();
     if (HasName && ND->getFormalLinkage() == InternalLinkage) {
       S.Diag(ND->getLocation(), diag::err_export_internal) << ND;
       if (BlockStart.isValid())
         S.Diag(BlockStart, diag::note_export);
       return false;
     }
   }
 
-  // C++2a [module.interface]p5:
+  // C++20 [module.interface]p5:
   //   all entities to which all of the using-declarators ultimately refer
   //   shall have been introduced with a name having external linkage
   if (auto *USD = dyn_cast<UsingShadowDecl>(D)) {
     NamedDecl *Target = USD->getUnderlyingDecl();
     Linkage Lk = Target->getFormalLinkage();
     if (Lk == InternalLinkage || Lk == ModuleLinkage) {
       S.Diag(USD->getLocation(), diag::err_export_using_internal)
           << (Lk == InternalLinkage ? 0 : 1) << Target;

clang/test/CXX/module/module.interface/p1-p2615r1.cpp

+// RUN: %clang_cc1 -std=c++20 %s -emit-module-interface -o p2615r1.pcm \
+// RUN: -verify -pedantic-errors
+
+export module P2615R1;
+
+export template<class T1, class T2, int I>
+class A { };
+
+export template<class T, int I>
+class A<T, T*, I> { }; // expected-error {{partial class specialization 'A<T, T *, I>' cannot be exported}}
+
+export template<class T1, class T2, int I>
+struct B { };
+
+export template<class T, int I>
+struct B<T, T*, I> { }; // expected-error {{partial class specialization 'B<T, T *, I>' cannot be exported}}
+
+export template<class T1, class T2, int I>
+T1 C;
+
+export template<class T, int I>
+T C<T, T*, I>;  // expected-error {{partial variable specialization 'C<T, T *, I>' cannot be exported}}

clang/test/CXX/temp/temp.explicit/p2-p2615r1.cpp

+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify -pedantic-errors
+
+export module P2615R1;
+
+export template<typename T> void fA (T) { };
+
+export template<>
+void fA<int> (int) { }; // expected-error {{explicit specialization 'fA<int>' cannot be exported}}
+
+export template<typename T>
+class B { };
+
+export template<>
+class B<int> { }; // expected-error {{explicit specialization 'B<int>' cannot be exported}}
+
+export template<typename T> T C;
+
+export template<>
+int C<int>;  // expected-error {{explicit specialization 'C<int>' cannot be exported}}
+

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

-// RUN: rm -rf %t
-// RUN: mkdir %t
-// RUN: split-file %s %t
-//
-// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/var_def.cppm -o %t/var_def.pcm
-// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/reexport1.cppm -o %t/reexport1.pcm
-// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/reexport2.cppm -o %t/reexport2.pcm
-// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/use.cppm -fsyntax-only -verify
-
-//--- use.cppm
-import reexport1;
-import reexport2;
-
-auto foo = zero<Int>;
-auto bar = zero<int*>;
-auto baz = zero<int>;
-
-template <class T> constexpr T zero = 0; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
-                                         // expected-note@* {{previous}}
-template <> constexpr Int zero<Int> = {0}; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
-                                           // expected-note@* {{previous}}
-template <class T> constexpr T* zero<T*> = nullptr; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
-                                                    // expected-note@* {{previous}}
-
-template <> constexpr int** zero<int**> = nullptr; // ok, new specialization.
-template <class T> constexpr T** zero<T**> = nullptr; // ok, new partial specilization.
-
-//--- var_def.cppm
-export module var_def;
-
-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;
-
-//--- reexport1.cppm
-export module reexport1;
-export import var_def;
-
-//--- reexport2.cppm
-export module reexport2;
-export import var_def;

clang/test/Modules/pr59780.cppm

-// https://github.com/llvm/llvm-project/issues/59780
-//
-// RUN: rm -rf %t
-// RUN: mkdir %t
-// RUN: split-file %s %t
-//
-// RUN: %clang_cc1 -std=c++20 %t/a.cppm -triple %itanium_abi_triple -emit-module-interface -o %t/a.pcm
-// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fprebuilt-module-path=%t -S \
-// RUN:     -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %t/use.cpp
-// RUN: %clang_cc1 -std=c++20 %t/a.pcm -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %t/a.cppm
-
-//--- a.cppm
-export module a;
-
-export template<typename T>
-int x = 0;
-
-export template<>
-int x<int> = 0;
-
-export template<typename T>
-struct Y {
-    static int value;
-};
-
-template <typename T>
-int Y<T>::value = 0;
-
-export template<>
-struct Y<int> {
-    static int value;
-};
-
-int Y<int>::value = 0;
-
-// CHECK-NOT: @_ZW1a1xIiE = {{.*}}external{{.*}}global
-// CHECK-NOT: @_ZNW1a1YIiE5valueE = {{.*}}external{{.*}}global
-
-//--- use.cpp
-import a;
-int foo() {
-    return x<int> + Y<int>::value;
-}
-
-// CHECK: @_ZW1a1xIiE = {{.*}}external{{.*}}global
-// CHECK: @_ZNW1a1YIiE5valueE = {{.*}}external{{.*}}global

clang/test/Modules/pr60890.cppm

 export module a;
  
 export template<typename T>
 struct a {
   friend void aa(a x) requires(true) {}
   void aaa() requires(true) {}
 };
 
-export template struct a<double>;
-
 export template<typename T>
 void foo(T) requires(true) {}
 
 export template void foo<double>(double);
 
 export template <typename T>
 class A {
   friend void foo<>(A);

clang/test/Modules/pr62796.cppm

struct DefaultStrategy
 
     constexpr unsigned long Compute(Number<10ul>, auto strategy)
   {
     return strategy(Recursive(10ul));
   }
 
   template<unsigned long N, typename Strategy = DefaultStrategy>
   constexpr unsigned long Cache = Compute(Number<N>{}, Strategy{});
-
-    template constexpr unsigned long Cache<10ul>;
 }
 
 //--- Use.cpp
 // expected-no-diagnostics
 import Fibonacci.Cache;
 
 constexpr bool value = Fibonacci::Cache<10ul> == 55;
 
 static_assert(value == true, "");

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

 export module foo;
 export using ::foo;
 export using ::foo3;
 
 export template <typename T>
 void foo4() {
 }
 
-export template <>
-void foo4<int>() {
-}
-
 //--- Use.cpp
 import foo;
 void use() {
   foo<short>();
   foo<int>();
   foo2<short>(); // expected-error {{missing '#include'; 'foo2' must be declared before it is used}}
                  // expected-note@* {{declaration here is not visible}}
   foo2<int>();   // expected-error {{missing '#include'; 'foo2' must be declared before it is used}}
                  // expected-note@* {{declaration here is not visible}}
   foo3<short>();
   foo3<int>();
 
   foo4<short>();
   foo4<int>();
 }

clang/unittests/Serialization/VarDeclConstantInitTest.cpp

export namespace Fibonacci
   constexpr unsigned long Compute(Number<10ul>, auto strategy)
   {
     return strategy(Recursive(10ul));
   }
 
   template<unsigned long N, typename Strategy = DefaultStrategy>
   constexpr unsigned long Cache = Compute(Number<N>{}, Strategy{});
 
-  template constexpr unsigned long Cache<10ul>;
 }
   )cpp");
 
   IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
       CompilerInstance::createDiagnostics(new DiagnosticOptions());
   CreateInvocationOptions CIOpts;
   CIOpts.Diags = Diags;
   CIOpts.VFS = llvm::vfs::createPhysicalFileSystem();
export namespace Fibonacci
   ASSERT_TRUE(Instance.ExecuteAction(Action));
   ASSERT_FALSE(Diags->hasErrorOccurred());
 
   std::string DepArg =
       llvm::Twine("-fmodule-file=Fibonacci.Cache=" + CacheBMIPath).str();
   std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCodeWithArgs(
       R"cpp(
 import Fibonacci.Cache;
+template constexpr unsigned long Fibonacci::Cache<10ul>;
         )cpp",
       /*Args=*/{"-std=c++20", DepArg.c_str()});
 
   using namespace clang::ast_matchers;
   ASTContext &Ctx = AST->getASTContext();
   const auto *cached = selectFirst<VarDecl>(
       "Cache",
       match(varDecl(isTemplateInstantiation(), hasName("Cache")).bind("Cache"),

clang/www/cxx_status.html

</tr>
         <td class="partial" align="center">Partial</td>
       </tr>
       <tr>
         <td><a href="https://wg21.link/p1815r2">P1815R2</a></td>
         <td class="partial" align="center">Partial</td>
       </tr>
       <tr> <!-- from Kona 2022 -->
         <td><a href="https://wg21.link/P2615R1">P2615R1</a> (<a href="#dr">DR</a>)</td>
-        <td class="none" align="center">No</td>
+        <td class="none" align="center">Clang 17</td>
       </tr>
       <tr> <!-- from Issaquah 2023 -->
         <td><a href="https://wg21.link/P2788R0">P2788R0</a> (<a href="#dr">DR</a>)</td>
         <td class="none" align="center">No</td>
       </tr>
     <tr>
       <td>Coroutines</td>
       <td><a href="https://wg21.link/p0912r5">P0912R5</a></td>