[C++20] [Modules] Treat module linkage as formal linkage only
authorChuanqi Xu <yedeng.yd@linux.alibaba.com>
Mon, 13 Mar 2023 07:43:08 +0000 (15:43 +0800)
committerChuanqi Xu <yedeng.yd@linux.alibaba.com>
Mon, 13 Mar 2023 07:54:40 +0000 (15:54 +0800)
Close https://github.com/llvm/llvm-project/issues/61321

There are two linkage modes in clang now: one for internal linkage and
one for formal linkage. The internal linkage is for the implementation
details and the formal linkage is for the consistency with the C++
standard.

Since we previously implemented the strong ownership for modules, the
module linkage is not meaningful for linkers any more. So the module
linkage should only be used for formal linkage.

clang/include/clang/AST/Decl.h
clang/lib/AST/Decl.cpp
clang/test/Modules/inconsistent-export.cppm [new file with mode: 0644]
clang/unittests/AST/DeclTest.cpp

index e213c5f..0585fda 100644 (file)
@@ -395,9 +395,7 @@ public:
 
   /// Get the linkage from a semantic point of view. Entities in
   /// anonymous namespaces are external (in c++98).
-  Linkage getFormalLinkage() const {
-    return clang::getFormalLinkage(getLinkageInternal());
-  }
+  Linkage getFormalLinkage() const;
 
   /// True if this decl has external linkage.
   bool hasExternalFormalLinkage() const {
index 240e744..484a4a2 100644 (file)
@@ -605,20 +605,6 @@ static LinkageInfo getInternalLinkageFor(const NamedDecl *D) {
 }
 
 static LinkageInfo getExternalLinkageFor(const NamedDecl *D) {
-  // C++ [basic.link]p4.8:
-  //   - if the declaration of the name is attached to a named module and is not
-  //   exported
-  //     the name has module linkage;
-  //
-  // [basic.namespace.general]/p2
-  //   A namespace is never attached to a named module and never has a name with
-  //   module linkage.
-  if (isInModulePurview(D) &&
-      !isExportedFromModuleInterfaceUnit(
-          cast<NamedDecl>(D->getCanonicalDecl())) &&
-      !isa<NamespaceDecl>(D))
-    return LinkageInfo(ModuleLinkage, DefaultVisibility, false);
-
   return LinkageInfo::external();
 }
 
@@ -1165,6 +1151,29 @@ Linkage NamedDecl::getLinkageInternal() const {
       .getLinkage();
 }
 
+/// Get the linkage from a semantic point of view. Entities in
+/// anonymous namespaces are external (in c++98).
+Linkage NamedDecl::getFormalLinkage() const {
+  Linkage InternalLinkage = getLinkageInternal();
+
+  // C++ [basic.link]p4.8:
+  //   - if the declaration of the name is attached to a named module and is not
+  //   exported
+  //     the name has module linkage;
+  //
+  // [basic.namespace.general]/p2
+  //   A namespace is never attached to a named module and never has a name with
+  //   module linkage.
+  if (isInModulePurview(this) &&
+      InternalLinkage == ExternalLinkage &&
+      !isExportedFromModuleInterfaceUnit(
+          cast<NamedDecl>(this->getCanonicalDecl())) &&
+      !isa<NamespaceDecl>(this))
+    InternalLinkage = ModuleLinkage;
+
+  return clang::getFormalLinkage(InternalLinkage);
+}
+
 LinkageInfo NamedDecl::getLinkageAndVisibility() const {
   return LinkageComputer{}.getDeclLinkageAndVisibility(this);
 }
diff --git a/clang/test/Modules/inconsistent-export.cppm b/clang/test/Modules/inconsistent-export.cppm
new file mode 100644 (file)
index 0000000..5e94d2b
--- /dev/null
@@ -0,0 +1,46 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/m-a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-module-interface -o %t/m-b.pcm \
+// RUN:     -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/m.cppm -emit-module-interface -o %t/m.pcm \
+// RUN:     -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/use.cppm -fprebuilt-module-path=%t -emit-obj
+
+//--- a.cppm
+export module m:a;
+namespace n {
+export class a {
+public:
+    virtual ~a() {}
+};
+}
+
+//--- b.cppm
+export module m:b;
+namespace n {
+class a;
+}
+
+//--- m.cppm
+export module m;
+export import :a;
+export import :b;
+
+//--- use.cppm
+// expected-no-diagnostics
+export module u;
+export import m;
+
+struct aa : public n::a {
+    aa() {}
+};
+auto foo(n::a*) {
+    return;
+}
+
+void use() {
+    n::a _;
+}
index 119551b..a653511 100644 (file)
@@ -232,16 +232,16 @@ TEST(Decl, ModuleAndInternalLinkage) {
   const auto *f = selectFirst<FunctionDecl>(
       "f", match(functionDecl(hasName("f")).bind("f"), Ctx));
 
-  EXPECT_EQ(a->getLinkageInternal(), InternalLinkage);
-  EXPECT_EQ(f->getLinkageInternal(), InternalLinkage);
+  EXPECT_EQ(a->getFormalLinkage(), InternalLinkage);
+  EXPECT_EQ(f->getFormalLinkage(), 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);
+  EXPECT_EQ(b->getFormalLinkage(), ModuleLinkage);
+  EXPECT_EQ(g->getFormalLinkage(), ModuleLinkage);
 
   AST = tooling::buildASTFromCodeWithArgs(
       Code.code(), /*Args=*/{"-std=c++20"});
@@ -250,15 +250,15 @@ TEST(Decl, ModuleAndInternalLinkage) {
   f = selectFirst<FunctionDecl>(
       "f", match(functionDecl(hasName("f")).bind("f"), CtxTS));
 
-  EXPECT_EQ(a->getLinkageInternal(), InternalLinkage);
-  EXPECT_EQ(f->getLinkageInternal(), InternalLinkage);
+  EXPECT_EQ(a->getFormalLinkage(), InternalLinkage);
+  EXPECT_EQ(f->getFormalLinkage(), InternalLinkage);
 
   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);
+  EXPECT_EQ(b->getFormalLinkage(), ModuleLinkage);
+  EXPECT_EQ(g->getFormalLinkage(), ModuleLinkage);
 }
 
 TEST(Decl, GetNonTransparentDeclContext) {