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

Nathan Sidwell via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 28 12:33:13 PST 2022


urnathan added a comment.

In D120397#3340256 <https://reviews.llvm.org/D120397#3340256>, @erichkeane wrote:

> Linkage stuff is where I get lost quickly, hopefully @urnathan can comment here.  Codewise I think this looks right.

looks right to me too -- take the linkage from the most general template.



================
Comment at: clang/test/Modules/inconsist-export-template.cpp:1-2
+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
+// expected-no-diagnostics
+export module m;
----------------
dblaikie wrote:
> This test doesn't appear to test anything - it verifies that this file produces no diagnostics, but not that it has any other/particular behavior.
> 
> Should this be testing codegen to verify that the correct linkage was used on the resulting IR functions?
> 
> Are there other ways of observing the particular language-level linkage of the entities to confirm it's as expected?
yes, this should be a codegen test (clang/test/CodeGenCXX/Modules/cxx20-$something.cpp?


================
Comment at: clang/test/Modules/inconsist-export-template.cpp:19-23
+// FIXME: We should reject following specialization,
+// since it tries to export a name which is already introduced.
+export template <>
+void f1<int>() {
+
----------------
I don't think we should be testing for ill-formed code here.  We want to verify that explicit instantiations, explicit specializations and implicit instantiations all get the expected linkage -- both external linkage on exported entities, module linkage on non-exported module-purview entities.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120397/new/

https://reviews.llvm.org/D120397



More information about the cfe-commits mailing list