[clang] 0c4f0bf - [C++20] [Modules] Only allow redeclarations in partitions if they are in the same module

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-10-08T17:44:04+08:00
New Revision: 0c4f0bf40d17d516aff54f6cea79b69101085799

URL: https://github.com/llvm/llvm-project/commit/0c4f0bf40d17d516aff54f6cea79b69101085799
DIFF: https://github.com/llvm/llvm-project/commit/0c4f0bf40d17d516aff54f6cea79b69101085799.diff

LOG: [C++20] [Modules] Only allow redeclarations in partitions if they are in the same module

Closes https://github.com/llvm/llvm-project/issues/58196.

The root cause for the problem is an oversight in
https://reviews.llvm.org/D127624, which allows the redeclarations in
partitions. However, we took a mistake there that we should only allow
it if the redeclarations in the one same module instead of return
directly if either the redeclaration lives in a partition. The original
implementation makes no sense and I believe it was an oversight.

Added: 
    clang/test/Modules/inconsistent-deduction-guide-linkage.cppm

Modified: 
    clang/lib/Sema/SemaDecl.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 760cf7fd805c..1bf959a35178 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1660,9 +1660,11 @@ bool Sema::CheckRedeclarationModuleOwnership(NamedDecl *New, NamedDecl *Old) {
 
   // 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();
+  if (NewM && OldM &&
+      (NewM->isModulePartition() || OldM->isModulePartition()) &&
+      NewM->getPrimaryModuleInterfaceName() ==
+          OldM->getPrimaryModuleInterfaceName())
+    return false;
 
   bool NewIsModuleInterface = NewM && NewM->isModulePurview();
   bool OldIsModuleInterface = OldM && OldM->isModulePurview();

diff  --git a/clang/test/Modules/inconsistent-deduction-guide-linkage.cppm b/clang/test/Modules/inconsistent-deduction-guide-linkage.cppm
new file mode 100644
index 000000000000..abcbec07f97d
--- /dev/null
+++ b/clang/test/Modules/inconsistent-deduction-guide-linkage.cppm
@@ -0,0 +1,49 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -I%t -emit-module-interface -o %t/B.pcm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only %t/A.cppm -I%t -fprebuilt-module-path=%t -verify
+//
+// RUN: %clang_cc1 -std=c++20 %t/D.cppm -I%t -emit-module-interface -o %t/D.pcm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only %t/D-part.cppm -I%t -fprebuilt-module-path=%t -verify
+
+//--- A.cppm
+module;
+export module baz:A;
+import B;
+#include "C.h"
+
+//--- B.cppm
+module;
+
+#include "C.h"
+export module B;
+
+//--- C.h
+namespace foo {
+  template<class T, class U> struct bar { // expected-error {{declaration of 'bar' in module baz:A follows declaration in the global module}} // expected-note {{previous declaration is here}}
+      template<class, class> bar(T, U);
+  };
+  template<class T, class U> bar(T, U) -> bar<T, U>; // expected-error {{declaration of '<deduction guide for bar>' in module baz:A follows declaration in the global module}} // expected-note {{previous declaration is here}}
+}
+
+//--- D.cppm
+// Tests that it is still problematic if they are in one module.
+module;
+#include "E.h"
+export module D;
+
+//--- D-part.cppm
+export module D:part;
+import D;
+#include "E.h"
+
+//--- E.h
+// another file for simpler diagnostics.
+namespace foo {
+  template<class T, class U> struct bar { // expected-error {{declaration of 'bar' in module D:part follows declaration in the global module}} // expected-note {{previous declaration is here}}
+      template<class, class> bar(T, U);
+  };
+  template<class T, class U> bar(T, U) -> bar<T, U>; // expected-error {{declaration of '<deduction guide for bar>' in module D:part follows declaration in the global module}} // expected-note {{previous declaration is here}}
+}