iains created this revision.
Herald added a project: All.
iains added reviewers: urnathan, ChuanqiXu.
iains published this revision for review.
iains added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
this is really a bug fix, rather than a new feature - the current impl was not
general enough.
The existing provision is not sufficient, it did not allow for the cases
where an implementation partition includes the primary module interface,
or for the case that an exported interface partition is contains a decl
that is then implemented in a regular implementation unit.
It is somewhat unfortunate that we have to compare top level module names
to achieve this, since built modules are not necessarily available.
TODO: It might be useful to cache a hash of the primary module name if
this test proves to be a significant load.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D127624
Files:
clang/lib/Sema/SemaDecl.cpp
clang/test/Modules/cxx20-partition-redeclarations.cpp
Index: clang/test/Modules/cxx20-partition-redeclarations.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/cxx20-partition-redeclarations.cpp
@@ -0,0 +1,55 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 A-intf-part.cpp -emit-module-interface \
+// RUN: -o A-PubPart.pcm
+// RUN: %clang_cc1 -std=c++20 A-interface.cpp -emit-module-interface \
+// RUN: -fmodule-file=A-PubPart.pcm -o A.pcm
+
+// RUN: %clang_cc1 -std=c++20 A-impl-top.cpp -fsyntax-only -fmodule-file=A.pcm
+// RUN: %clang_cc1 -std=c++20 A-impl-part.cpp -fsyntax-only -fmodule-file=A.pcm
+// RUN: %clang_cc1 -std=c++20 A-impl-1.cpp -fsyntax-only -fmodule-file=A.pcm
+// RUN: %clang_cc1 -std=c++20 A-impl-2.cpp -fsyntax-only -fmodule-file=A.pcm
+
+//--- A-interface.cpp
+export module A;
+
+export import :PubPart;
+
+export void do_something();
+
+void helper1();
+void helper3();
+
+//--- A-intf-part.cpp
+export module A:PubPart;
+
+void helper2();
+
+//--- A-impl-top.cpp
+
+module A;
+
+void do_something() {
+ helper1();
+ helper2();
+ helper3();
+}
+
+//--- A-impl-part.cpp
+module A:Secret;
+
+import A;
+
+void helper3() {}
+
+//--- A-impl-1.cpp
+module A;
+
+void helper1() {}
+
+//--- A-impl-2.cpp
+module A;
+
+void helper2() {}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1625,22 +1625,20 @@
Module *NewM = New->getOwningModule();
Module *OldM = Old->getOwningModule();
- if (NewM && NewM->Kind == Module::PrivateModuleFragment)
+ if (NewM && NewM->isPrivateModule())
NewM = NewM->Parent;
- if (OldM && OldM->Kind == Module::PrivateModuleFragment)
+ if (OldM && OldM->isPrivateModule())
OldM = OldM->Parent;
- // If we have a decl in a module partition, it is part of the containing
- // module (which is the only thing that can be importing it).
- if (NewM && OldM &&
- (OldM->Kind == Module::ModulePartitionInterface ||
- OldM->Kind == Module::ModulePartitionImplementation)) {
- return false;
- }
-
if (NewM == OldM)
return false;
+ // Partitions are part of the module, but a partition could import another
+ // module, so verify that the PMIs agree.
+ if (NewM && OldM && (NewM->isModulePartition() || OldM->isModulePartition()))
+ return NewM->getPrimaryModuleInterfaceName() ==
+ OldM->getPrimaryModuleInterfaceName();
+
bool NewIsModuleInterface = NewM && NewM->isModulePurview();
bool OldIsModuleInterface = OldM && OldM->isModulePurview();
if (NewIsModuleInterface || OldIsModuleInterface) {
Index: clang/test/Modules/cxx20-partition-redeclarations.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/cxx20-partition-redeclarations.cpp
@@ -0,0 +1,55 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 A-intf-part.cpp -emit-module-interface \
+// RUN: -o A-PubPart.pcm
+// RUN: %clang_cc1 -std=c++20 A-interface.cpp -emit-module-interface \
+// RUN: -fmodule-file=A-PubPart.pcm -o A.pcm
+
+// RUN: %clang_cc1 -std=c++20 A-impl-top.cpp -fsyntax-only -fmodule-file=A.pcm
+// RUN: %clang_cc1 -std=c++20 A-impl-part.cpp -fsyntax-only -fmodule-file=A.pcm
+// RUN: %clang_cc1 -std=c++20 A-impl-1.cpp -fsyntax-only -fmodule-file=A.pcm
+// RUN: %clang_cc1 -std=c++20 A-impl-2.cpp -fsyntax-only -fmodule-file=A.pcm
+
+//--- A-interface.cpp
+export module A;
+
+export import :PubPart;
+
+export void do_something();
+
+void helper1();
+void helper3();
+
+//--- A-intf-part.cpp
+export module A:PubPart;
+
+void helper2();
+
+//--- A-impl-top.cpp
+
+module A;
+
+void do_something() {
+ helper1();
+ helper2();
+ helper3();
+}
+
+//--- A-impl-part.cpp
+module A:Secret;
+
+import A;
+
+void helper3() {}
+
+//--- A-impl-1.cpp
+module A;
+
+void helper1() {}
+
+//--- A-impl-2.cpp
+module A;
+
+void helper2() {}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1625,22 +1625,20 @@
Module *NewM = New->getOwningModule();
Module *OldM = Old->getOwningModule();
- if (NewM && NewM->Kind == Module::PrivateModuleFragment)
+ if (NewM && NewM->isPrivateModule())
NewM = NewM->Parent;
- if (OldM && OldM->Kind == Module::PrivateModuleFragment)
+ if (OldM && OldM->isPrivateModule())
OldM = OldM->Parent;
- // If we have a decl in a module partition, it is part of the containing
- // module (which is the only thing that can be importing it).
- if (NewM && OldM &&
- (OldM->Kind == Module::ModulePartitionInterface ||
- OldM->Kind == Module::ModulePartitionImplementation)) {
- return false;
- }
-
if (NewM == OldM)
return false;
+ // Partitions are part of the module, but a partition could import another
+ // module, so verify that the PMIs agree.
+ if (NewM && OldM && (NewM->isModulePartition() || OldM->isModulePartition()))
+ return NewM->getPrimaryModuleInterfaceName() ==
+ OldM->getPrimaryModuleInterfaceName();
+
bool NewIsModuleInterface = NewM && NewM->isModulePurview();
bool OldIsModuleInterface = OldM && OldM->isModulePurview();
if (NewIsModuleInterface || OldIsModuleInterface) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
