[PATCH] D122413: [C++20][Modules] Limit ModuleInternalLinkage to modules-ts.

[Iain Sandoe via Phabricator via cfe-commits]

iains created this revision.
Herald added a project: All.
iains added reviewers: urnathan, dblaikie, Bigcheese, rsmith, ChuanqiXu.
iains published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

At present, we are generating wrong code for C++20 modules entities which
should have internal linkage.  This is because we are assigning
'ModuleInternalLinkage' unconditionally to such entities.  However this mode
is only applicable to the modules-ts.

This change makes the special linkage mode conditional on fmodules-ts and
adds a unit test to verify that we generate the correct linkage.

Currently, static variables and functions in module purview are emitted into
object files as external. On some platforms, lambdas are emitted as global
weak defintions (on Windows this causes a mangler crash).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122413

Files:
  clang/lib/AST/Decl.cpp
  clang/unittests/AST/DeclTest.cpp


Index: clang/unittests/AST/DeclTest.cpp
===================================================================
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -194,3 +194,50 @@
   EXPECT_EQ(TemplateF->getLinkageInternal(),
             SpecializedF->getLinkageInternal());
 }
+
+TEST(Decl, ModuleAndInternalLinkage) {
+  llvm::Annotations Code(R"(
+    export module M;
+    static int a;
+    static int f(int x);
+
+    int b;
+    int g(int x);)");
+
+  auto AST =
+      tooling::buildASTFromCodeWithArgs(Code.code(), /*Args=*/{"-std=c++20"});
+  ASTContext &Ctx = AST->getASTContext();
+
+  const auto *a =
+      selectFirst<VarDecl>("a", match(varDecl(hasName("a")).bind("a"), Ctx));
+  const auto *f = selectFirst<FunctionDecl>(
+      "f", match(functionDecl(hasName("f")).bind("f"), Ctx));
+
+  EXPECT_EQ(a->getLinkageInternal(), InternalLinkage);
+  EXPECT_EQ(f->getLinkageInternal(), InternalLinkage);
+
+  const auto *b =
+      selectFirst<VarDecl>("b", match(varDecl(hasName("b")).bind("b"), Ctx));
+  const auto *g = selectFirst<FunctionDecl>(
+      "g", match(functionDecl(hasName("g")).bind("g"), Ctx));
+
+  EXPECT_EQ(b->getLinkageInternal(), ModuleLinkage);
+  EXPECT_EQ(g->getLinkageInternal(), ModuleLinkage);
+
+  AST = tooling::buildASTFromCodeWithArgs(
+      Code.code(), /*Args=*/{"-std=c++20", "-fmodules-ts"});
+  ASTContext &CtxTS = AST->getASTContext();
+  a = selectFirst<VarDecl>("a", match(varDecl(hasName("a")).bind("a"), CtxTS));
+  f = selectFirst<FunctionDecl>(
+      "f", match(functionDecl(hasName("f")).bind("f"), CtxTS));
+
+  EXPECT_EQ(a->getLinkageInternal(), ModuleInternalLinkage);
+  EXPECT_EQ(f->getLinkageInternal(), ModuleInternalLinkage);
+
+  b = selectFirst<VarDecl>("b", match(varDecl(hasName("b")).bind("b"), CtxTS));
+  g = selectFirst<FunctionDecl>(
+      "g", match(functionDecl(hasName("g")).bind("g"), CtxTS));
+
+  EXPECT_EQ(b->getLinkageInternal(), ModuleLinkage);
+  EXPECT_EQ(g->getLinkageInternal(), ModuleLinkage);
+}
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -596,11 +596,12 @@
 }
 
 static LinkageInfo getInternalLinkageFor(const NamedDecl *D) {
-  // Internal linkage declarations within a module interface unit are modeled
-  // as "module-internal linkage", which means that they have internal linkage
-  // formally but can be indirectly accessed from outside the module via inline
-  // functions and templates defined within the module.
-  if (isInModulePurview(D))
+  // (for the modules ts) Internal linkage declarations within a module
+  // interface unit are modeled as "module-internal linkage", which means that
+  // they have internal linkage formally but can be indirectly accessed from
+  // outside the module via inline functions and templates defined within the
+  // module.
+  if (isInModulePurview(D) && D->getASTContext().getLangOpts().ModulesTS)
     return LinkageInfo(ModuleInternalLinkage, DefaultVisibility, false);
 
   return LinkageInfo::internal();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D122413.417965.patch
Type: text/x-patch
Size: 3092 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220324/f008a3f2/attachment.bin>