[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: urnathan, iains, rsmith, aaron.ballman, erichkeane.
ChuanqiXu added a project: clang.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.

Before the patch, the compiler would crash for the test due to inconsistent linkage.

This patch tries to avoid it by make the linkage consistent for template and its specialization. After the patch, the behavior of compiler would be partially correct for the case.
The correct one is:

  export template<class T>
  void f() {}
  
  template<>
  void f<int>() {}

In this case, the linkage for both declaration should be external (the wording I get by consulting in WG21 is "the linkage for name `f` should be external").

And for the case:

  template<class T>
  void f() {}
  
  export template<>
  void f<int>() {}

Compiler should reject it. This isn't done now. I marked it as FIXME in the test. After all, this patch would stop a crash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120397

Files:
  clang/lib/AST/Decl.cpp
  clang/test/Modules/inconsist-export-template.cpp


Index: clang/test/Modules/inconsist-export-template.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/inconsist-export-template.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
+// expected-no-diagnostics
+export module m;
+export template <class T>
+void f() {
+
+}
+
+template <>
+void f<int>() {
+
+}
+
+template <class T>
+void f1() {
+
+}
+
+// FIXME: We should reject following specialization,
+// since it tries to export a name which is already introduced.
+export template <>
+void f1<int>() {
+
+}
+
+export template<class T>
+class C {
+
+};
+
+template<>
+class C<int> {
+
+};
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -391,11 +391,18 @@
   bool considerVisibility =
     shouldConsiderTemplateVisibility(fn, specInfo);
 
-  // Merge information from the template parameters.
   FunctionTemplateDecl *temp = specInfo->getTemplate();
-  LinkageInfo tempLV =
+
+  // Merge information from the template declaration.
+  LinkageInfo tempLV = getLVForDecl(temp, computation);
+  // The linkage of the specialization should be consistent with the
+  // template declaration.
+  LV.setLinkage(tempLV.getLinkage());
+
+  // Merge information from the template parameters.
+  LinkageInfo paramsLV =
     getLVForTemplateParameterList(temp->getTemplateParameters(), computation);
-  LV.mergeMaybeWithVisibility(tempLV, considerVisibility);
+  LV.mergeMaybeWithVisibility(paramsLV, considerVisibility);
 
   // Merge information from the template arguments.
   const TemplateArgumentList &templateArgs = *specInfo->TemplateArguments;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D120397.410743.patch
Type: text/x-patch
Size: 1735 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220223/5af2ad11/attachment-0001.bin>