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

[iains]

Summary

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.

[iains]

this is really a bug fix, rather than a new feature - the current impl was not general enough.

[ChuanqiXu]

LGTM. Yeah, it might be painful to compare string all the time. Let's do it when refactoring.

clang/lib/Sema/SemaDecl.cpp

if (New->getFriendObjectKind() &&
     New->setLocalOwningModule(Old->getOwningModule());
     makeMergedDefinitionVisible(New);
     return false;
   }
 
   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) {
     // C++ Modules TS [basic.def.odr] 6.2/6.7 [sic]:
     //   if a declaration of D [...] appears in the purview of a module, all
     //   other such declarations shall appear in the purview of the same module
     Diag(New->getLocation(), diag::err_mismatched_owning_module)
       << New

clang/test/Modules/cxx20-partition-redeclarations.cpp

+// 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() {}