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;


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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to