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 || 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 CheckRedefinitionInModule(const NamedDecl *New, + const NamedDecl *Old) const; void DiagnoseAmbiguousLookup(LookupResult &Result); //@} 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::CheckRedefinitionInModule(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(this)->isReachable(New) && + const_cast(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(D) || isa(D) || 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(Previous.getRepresentativeDecl()); + auto *OldConcept = dyn_cast(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) && + CheckRedefinitionInModule(NewDecl, OldConcept)) { Diag(NewDecl->getLocation(), diag::err_redefinition) << NewDecl->getDeclName(); notePreviousDefinition(OldConcept, NewDecl->getLocation()); 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 them co_exist? Or will them be mutual from each other? +// 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 +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 +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 void foo() + requires same_as +{} + +//--- Use2.cpp +// expected-no-diagnostics +#include "foo.h" +import A; + +template void foo() + requires same_as +{} + +//--- Use3.cpp +// expected-no-diagnostics +import A; +#include "foo.h" + +template void foo() + requires same_as +{} + +//--- Use4.cpp +// expected-no-diagnostics +import A; +import B; +#include "foo.h" + +template void foo() + requires same_as +{} + +//--- C.cppm +// expected-no-diagnostics +module; +#include "foo.h" +export module C; +import A; +import B; + +template void foo() + requires same_as +{} + +//--- 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 +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 +concept same_as = __is_same(T, U); +template +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 void foo() + requires same_as +{} + +//--- Use6.cpp +// expected-no-diagnostics +import A; +import "foo.h"; + +template void foo() + requires same_as +{} + +//--- module.map +module "foo" { + export * + header "foo.h" +} + +//--- Use7.cpp +// expected-no-diagnostics +#include "foo.h" +import A; + +template void foo() + requires same_as +{} + +//--- Use8.cpp +// expected-no-diagnostics +import A; +#include "foo.h" + +template void foo() + requires same_as +{}