ChuanqiXu created this revision.
ChuanqiXu added a reviewer: ilya-biryukov.
ChuanqiXu added a project: clang.
Herald added a project: All.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.
I found this one when I try to add a C++20 Modules example for D130585
<https://reviews.llvm.org/D130585>. Then I found there are something to improve:
(1) When the declaration is shadowed by using decls, we could try to find its
targeted decl. This is fine since we would check Previous.isSingleResult()
before we process anything.
(2) When we complain about the redefinition, we need to check for the existence
of global module fragment. GMF is designed to make C++20 Named modules to be
compatible with headers.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D130614
Files:
clang/lib/Sema/SemaTemplate.cpp
clang/test/Modules/merge-concepts-in-GMF.cppm
Index: clang/test/Modules/merge-concepts-in-GMF.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/merge-concepts-in-GMF.cppm
@@ -0,0 +1,63 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/A.cppm -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/B.cppm -o %t/B.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/Use.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/Use2.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/Use3.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/Use4.cpp -verify -fsyntax-only
+
+//--- foo.h
+template <class T, class U>
+concept same_as = __is_same(T, U);
+
+//--- A.cppm
+module;
+#include "foo.h"
+export module A;
+export using ::same_as;
+
+//--- B.cppm
+module;
+#include "foo.h"
+export module B;
+export using ::same_as;
+
+//--- Use.cpp
+// expected-no-diagnostics
+import A;
+import B;
+
+template <class T> void foo()
+ requires same_as<T, int>
+{}
+
+//--- Use2.cpp
+// expected-no-diagnostics
+#include "foo.h"
+import A;
+
+template <class T> void foo()
+ requires same_as<T, int>
+{}
+
+//--- Use3.cpp
+// expected-no-diagnostics
+import A;
+#include "foo.h"
+
+template <class T> void foo()
+ requires same_as<T, int>
+{}
+
+//--- Use4.cpp
+// expected-no-diagnostics
+import A;
+import B;
+#include "foo.h"
+
+template <class T> void foo()
+ requires same_as<T, int>
+{}
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -8728,9 +8728,12 @@
if (Previous.empty())
return;
- auto *OldConcept = dyn_cast<ConceptDecl>(Previous.getRepresentativeDecl());
+ NamedDecl *Old = Previous.getRepresentativeDecl();
+ if (auto *USD = dyn_cast<UsingShadowDecl>(Old))
+ Old = USD->getTargetDecl();
+
+ auto *OldConcept = dyn_cast<ConceptDecl>(Old);
if (!OldConcept) {
- auto *Old = Previous.getRepresentativeDecl();
Diag(NewDecl->getLocation(), diag::err_redefinition_different_kind)
<< NewDecl->getDeclName();
notePreviousDefinition(Old, NewDecl->getLocation());
@@ -8746,7 +8749,11 @@
AddToScope = false;
return;
}
- if (hasReachableDefinition(OldConcept)) {
+ if (hasReachableDefinition(OldConcept) &&
+ // When the old decl lives in the GMF and the new decl is same with the
+ // old one, we should merge them instead of complaining.
+ !(OldConcept->getOwningModule() &&
+ OldConcept->getOwningModule()->isGlobalModule())) {
Diag(NewDecl->getLocation(), diag::err_redefinition)
<< NewDecl->getDeclName();
notePreviousDefinition(OldConcept, NewDecl->getLocation());
@@ -8758,7 +8765,7 @@
// Other decls (e.g. namespaces) also have this shortcoming.
return;
}
- Context.setPrimaryMergedDecl(NewDecl, OldConcept);
+ Context.setPrimaryMergedDecl(NewDecl, OldConcept->getCanonicalDecl());
}
/// \brief Strips various properties off an implicit instantiation
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
