[PATCH] D127624: [C++20][Modules] Allow for redeclarations in partitions.

[Iain Sandoe via Phabricator via cfe-commits]

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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D127624.436318.patch
Type: text/x-patch
Size: 2695 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220613/17450dd1/attachment.bin>