ChuanqiXu updated this revision to Diff 448504.
ChuanqiXu marked 3 inline comments as done.
ChuanqiXu added a comment.
Address comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130614/new/
https://reviews.llvm.org/D130614
Files:
clang/include/clang/Basic/Module.h
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/Modules/merge-concepts.cppm
Index: clang/test/Modules/merge-concepts.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/merge-concepts.cppm
@@ -0,0 +1,185 @@
+// 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 -fprebuilt-module-path=%t %t/Use.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use2.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use3.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use4.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/C.cppm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -I%t %t/D.cppm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -I%t %t/E.cppm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -I%t %t/F.cppm -verify -fsyntax-only
+//
+// Testing header units for coverity.
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/foo.h -o %t/foo.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -fmodule-file=%t/foo.pcm %t/Use5.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -fmodule-file=%t/foo.pcm %t/Use6.cpp -verify -fsyntax-only
+//
+// Testing with module map modules. It is unclear about the relation ship between Clang modules and
+// C++20 Named Modules. Will they coexist? Or will they be mutually exclusive?
+// The test here is for primarily coverity.
+//
+// RUN: rm -f %t/foo.pcm
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fprebuilt-module-path=%t \
+// RUN: -fmodule-map-file=%t/module.map %t/Use7.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fprebuilt-module-path=%t \
+// RUN: -fmodule-map-file=%t/module.map %t/Use7.cpp -verify -fsyntax-only
+// Testing module map modules with named modules.
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fmodule-map-file=%t/module.map \
+// RUN: %t/A.cppm -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fprebuilt-module-path=%t \
+// RUN: -fmodule-map-file=%t/module.map %t/Use7.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fprebuilt-module-path=%t \
+// RUN: -fmodule-map-file=%t/module.map %t/Use7.cpp -verify -fsyntax-only
+
+//
+//--- foo.h
+#ifndef FOO_H
+#define FOO_H
+template <class T, class U>
+concept same_as = __is_same(T, U);
+#endif
+
+// The compiler would warn if we include foo_h twice without guard.
+//--- redecl.h
+#ifndef REDECL_H
+#define REDECL_H
+template <class T, class U>
+concept same_as = __is_same(T, U);
+#endif
+
+//--- 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>
+{}
+
+//--- C.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module C;
+import A;
+import B;
+
+template <class T> void foo()
+ requires same_as<T, int>
+{}
+
+//--- D.cppm
+module;
+#include "foo.h"
+#include "redecl.h"
+export module D;
+export using ::same_as;
+
+// expected-error@* {{redefinition of 'same_as'}}
+// expected-note@* 1+{{previous definition is here}}
+
+//--- E.cppm
+module;
+#include "foo.h"
+export module E;
+export template <class T, class U>
+concept same_as = __is_same(T, U);
+
+// expected-error@* {{redefinition of 'same_as'}}
+// expected-note@* 1+{{previous definition is here}}
+
+//--- F.cppm
+export module F;
+template <class T, class U>
+concept same_as = __is_same(T, U);
+template <class T, class U>
+concept same_as = __is_same(T, U);
+
+// expected-error@* {{redefinition of 'same_as'}}
+// expected-note@* 1+{{previous definition is here}}
+
+//--- Use5.cpp
+// expected-no-diagnostics
+import "foo.h";
+import A;
+
+template <class T> void foo()
+ requires same_as<T, int>
+{}
+
+//--- Use6.cpp
+// expected-no-diagnostics
+import A;
+import "foo.h";
+
+template <class T> void foo()
+ requires same_as<T, int>
+{}
+
+//--- module.map
+module "foo" {
+ export *
+ header "foo.h"
+}
+
+//--- Use7.cpp
+// expected-no-diagnostics
+#include "foo.h"
+import A;
+
+template <class T> void foo()
+ requires same_as<T, int>
+{}
+
+//--- Use8.cpp
+// expected-no-diagnostics
+import A;
+#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
@@ -8724,7 +8724,7 @@
if (Previous.empty())
return;
- auto *OldConcept = dyn_cast<ConceptDecl>(Previous.getRepresentativeDecl());
+ auto *OldConcept = dyn_cast<ConceptDecl>(Previous.getRepresentativeDecl()->getUnderlyingDecl());
if (!OldConcept) {
auto *Old = Previous.getRepresentativeDecl();
Diag(NewDecl->getLocation(), diag::err_redefinition_different_kind)
@@ -8742,7 +8742,8 @@
AddToScope = false;
return;
}
- if (hasReachableDefinition(OldConcept)) {
+ if (hasReachableDefinition(OldConcept) &&
+ IsRedefinitionInModule(NewDecl, OldConcept)) {
Diag(NewDecl->getLocation(), diag::err_redefinition)
<< NewDecl->getDeclName();
notePreviousDefinition(OldConcept, NewDecl->getLocation());
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1719,6 +1719,80 @@
return false;
}
+// Check the redefinition in C++20 Modules.
+//
+// [basic.def.odr]p14:
+// For any definable item D with definitions in multiple translation units,
+// - if D is a non-inline non-templated function or variable, or
+// - if the definitions in different translation units do not satisfy the
+// following requirements,
+// the program is ill-formed; a diagnostic is required only if the definable
+// item is attached to a named module and a prior definition is reachable at
+// the point where a later definition occurs.
+// - Each such definition shall not be attached to a named module
+// ([module.unit]).
+// - Each such definition shall consist of the same sequence of tokens, ...
+// ...
+//
+// Return true if the redefinition is not allowed. Return false otherwise.
+bool Sema::IsRedefinitionInModule(const NamedDecl *New,
+ const NamedDecl *Old) const {
+ assert(getASTContext().isSameEntity(New, Old) &&
+ "New and Old are not the same definition, we should diagnostic it "
+ "immediately instead of checking it.");
+ assert(const_cast<Sema *>(this)->isReachable(New) &&
+ const_cast<Sema *>(this)->isReachable(Old) &&
+ "We shouldn't see unreachable definitions here.");
+
+ Module *NewM = New->getOwningModule();
+ Module *OldM = Old->getOwningModule();
+
+ // We only checks for named modules here. The header like modules is skipped.
+ // FIXME: This is not right if we import the header like modules in the module
+ // purview.
+ //
+ // For example, assuming "header.h" provides definition for `D`.
+ // ```C++
+ // //--- M.cppm
+ // export module M;
+ // import "header.h"; // or #include "header.h" but import it by clang modules
+ // actually.
+ //
+ // //--- Use.cpp
+ // import M;
+ // import "header.h"; // or uses clang modules.
+ // ```
+ //
+ // In this case, `D` has multiple definitions in multiple TU (M.cppm and
+ // Use.cpp) and `D` is attached to a named module `M`. The compiler should
+ // reject it. But the current implementation couldn't detect the case since we
+ // don't record the information about the importee modules.
+ //
+ // But this might not be painful in practice. Since the design of C++20 Named
+ // Modules suggests us to use headers in global module fragment instead of
+ // module purview.
+ if (NewM && NewM->isHeaderLikeModule())
+ NewM = nullptr;
+ if (OldM && OldM->isHeaderLikeModule())
+ OldM = nullptr;
+
+ if (!NewM && !OldM)
+ return true;
+
+ // [basic.def.odr]p14.3
+ // Each such definition shall not be attached to a named module
+ // ([module.unit]).
+ if ((NewM && NewM->isModulePurview()) || (OldM && OldM->isModulePurview()))
+ return true;
+
+ // Then New and Old lives in the same TU if their share one same module unit.
+ if (NewM)
+ NewM = NewM->getTopLevelModule();
+ if (OldM)
+ OldM = OldM->getTopLevelModule();
+ return OldM == NewM;
+}
+
static bool isUsingDecl(NamedDecl *D) {
return isa<UsingShadowDecl>(D) ||
isa<UnresolvedUsingTypenameDecl>(D) ||
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -4479,6 +4479,8 @@
bool CheckRedeclarationModuleOwnership(NamedDecl *New, NamedDecl *Old);
bool CheckRedeclarationExported(NamedDecl *New, NamedDecl *Old);
bool CheckRedeclarationInModule(NamedDecl *New, NamedDecl *Old);
+ bool IsRedefinitionInModule(const NamedDecl *New,
+ const NamedDecl *Old) const;
void DiagnoseAmbiguousLookup(LookupResult &Result);
//@}
Index: clang/include/clang/Basic/Module.h
===================================================================
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -525,6 +525,11 @@
Parent->SubModules.push_back(this);
}
+ /// Is this module have similar semantics as headers.
+ bool isHeaderLikeModule() const {
+ return isModuleMapModule() || isHeaderUnit();
+ }
+
/// Is this a module partition.
bool isModulePartition() const {
return Kind == ModulePartitionInterface ||
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
