diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index bc4a0df296d71..40cadd93158c6 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -448,7 +448,7 @@ class ASTContext : public RefCountedBase { llvm::DenseMap ModuleInitializers; /// This is the top-level (C++20) Named module we are building. - Module *CurrentCXXNamedModule = nullptr; + Module *CurrentCXXNamedModule = nullptr; static constexpr unsigned ConstantArrayTypesLog2InitSize = 8; static constexpr unsigned GeneralTypesLog2InitSize = 9; diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index e0dc6dab7b334..f7d5b3a83141a 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -644,6 +644,9 @@ class alignas(8) Decl { return getModuleOwnershipKind() > ModuleOwnershipKind::VisibleWhenImported; } + /// Whether this declaration comes from another module unit. + bool isInAnotherModuleUnit() const; + /// FIXME: Implement discarding declarations actually in global module /// fragment. See [module.global.frag]p3,4 for details. bool isDiscardedInGlobalModuleFragment() const { return false; } diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index bc6fb74df4d62..e869117a8ea66 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2364,9 +2364,6 @@ class Sema final { return Entity->getOwningModule(); } - // Determine whether the module M belongs to the current TU. - bool isModuleUnitOfCurrentTU(const Module *M) const; - /// Make a merged definition of an existing hidden definition \p ND /// visible at the specified location. void makeMergedDefinitionVisible(NamedDecl *ND); diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index ad1e940c4bda6..2307c33c5900c 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -1155,7 +1155,7 @@ ArrayRef ASTContext::getModuleInitializers(Module *M) { void ASTContext::setCurrentNamedModule(Module *M) { assert(M->isModulePurview()); assert(!CurrentCXXNamedModule && - "We should set named module for ASTContext for only once"); + "We should set named module for ASTContext for only once"); CurrentCXXNamedModule = M; } @@ -11935,9 +11935,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) { return false; // Variables in other module units shouldn't be forced to be emitted. - auto *VM = VD->getOwningModule(); - if (VM && VM->getTopLevelModule()->isModulePurview() && - VM->getTopLevelModule() != getCurrentNamedModule()) + if (VD->isInAnotherModuleUnit()) return false; // Variables that can be needed in other TUs are required. diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index f49945f434193..834beef49a444 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -30,6 +30,7 @@ #include "clang/Basic/IdentifierTable.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/LangOptions.h" +#include "clang/Basic/Module.h" #include "clang/Basic/ObjCRuntime.h" #include "clang/Basic/PartialDiagnostic.h" #include "clang/Basic/SourceLocation.h" @@ -1022,6 +1023,28 @@ bool Decl::isInExportDeclContext() const { return DC && isa(DC); } +bool Decl::isInAnotherModuleUnit() const { + auto *M = getOwningModule(); + + if (!M) + return false; + + M = M->getTopLevelModule(); + // FIXME: It is problematic if the header module lives in another module + // unit. Consider to fix this by techniques like + // ExternalASTSource::hasExternalDefinitions. + if (M->isHeaderLikeModule()) + return false; + + // A global module without parent implies that we're parsing the global + // module. So it can't be in another module unit. + if (M->isGlobalModule()) + return false; + + assert(M->isModulePurview() && "New module kind?"); + return M != getASTContext().getCurrentNamedModule(); +} + static Decl::Kind getKind(const Decl *D) { return D->getKind(); } static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); } diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp index e3a47334e2a2f..5a2a3616d1136 100644 --- a/clang/lib/Sema/SemaLookup.cpp +++ b/clang/lib/Sema/SemaLookup.cpp @@ -1905,14 +1905,11 @@ bool LookupResult::isReachableSlow(Sema &SemaRef, NamedDecl *D) { Module *DeclModule = SemaRef.getOwningModule(D); assert(DeclModule && "hidden decl has no owning module"); - // Entities in module map modules are reachable only if they're visible. - if (DeclModule->isModuleMapModule()) + // Entities in header like modules are reachable only if they're visible. + if (DeclModule->isHeaderLikeModule()) return false; - // If D comes from a module and SemaRef doesn't own a module, it implies D - // comes from another TU. In case SemaRef owns a module, we could judge if D - // comes from another TU by comparing the module unit. - if (SemaRef.isModuleUnitOfCurrentTU(DeclModule)) + if (!D->isInAnotherModuleUnit()) return true; // [module.reach]/p3: @@ -3893,7 +3890,7 @@ void Sema::ArgumentDependentLookup(DeclarationName Name, SourceLocation Loc, "bad export context"); // .. are attached to a named module M, do not appear in the // translation unit containing the point of the lookup.. - if (!isModuleUnitOfCurrentTU(FM) && + if (D->isInAnotherModuleUnit() && llvm::any_of(AssociatedClasses, [&](auto *E) { // ... and have the same innermost enclosing non-inline // namespace scope as a declaration of an associated entity diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp index a1ab013fcf5e8..67c6556e00ddf 100644 --- a/clang/lib/Sema/SemaModule.cpp +++ b/clang/lib/Sema/SemaModule.cpp @@ -1024,16 +1024,3 @@ void Sema::PopImplicitGlobalModuleFragment() { "left the wrong module scope, which is not global module fragment"); ModuleScopes.pop_back(); } - -bool Sema::isModuleUnitOfCurrentTU(const Module *M) const { - assert(M); - - Module *CurrentModuleUnit = getCurrentModule(); - - // If we are not in a module currently, M must not be the module unit of - // current TU. - if (!CurrentModuleUnit) - return false; - - return M->isSubModuleOf(CurrentModuleUnit->getTopLevelModule()); -} diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 135bf8053a964..7f3e78c89f57a 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -6543,23 +6543,20 @@ void Sema::AddOverloadCandidate( } // Functions with internal linkage are only viable in the same module unit. - if (auto *MF = Function->getOwningModule()) { - if (getLangOpts().CPlusPlusModules && !MF->isModuleMapModule() && - !isModuleUnitOfCurrentTU(MF)) { - /// FIXME: Currently, the semantics of linkage in clang is slightly - /// different from the semantics in C++ spec. In C++ spec, only names - /// have linkage. So that all entities of the same should share one - /// linkage. But in clang, different entities of the same could have - /// different linkage. - NamedDecl *ND = Function; - if (auto *SpecInfo = Function->getTemplateSpecializationInfo()) - ND = SpecInfo->getTemplate(); - - if (ND->getFormalLinkage() == Linkage::InternalLinkage) { - Candidate.Viable = false; - Candidate.FailureKind = ovl_fail_module_mismatched; - return; - } + if (getLangOpts().CPlusPlusModules && Function->isInAnotherModuleUnit()) { + /// FIXME: Currently, the semantics of linkage in clang is slightly + /// different from the semantics in C++ spec. In C++ spec, only names + /// have linkage. So that all entities of the same should share one + /// linkage. But in clang, different entities of the same could have + /// different linkage. + NamedDecl *ND = Function; + if (auto *SpecInfo = Function->getTemplateSpecializationInfo()) + ND = SpecInfo->getTemplate(); + + if (ND->getFormalLinkage() == Linkage::InternalLinkage) { + Candidate.Viable = false; + Candidate.FailureKind = ovl_fail_module_mismatched; + return; } } diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index c6ba58724e774..7a545214596ba 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -2851,8 +2851,7 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams, NewDefaultLoc = NewTypeParm->getDefaultArgumentLoc(); SawDefaultArgument = true; - if (!OldTypeParm->getOwningModule() || - isModuleUnitOfCurrentTU(OldTypeParm->getOwningModule())) + if (!OldTypeParm->getOwningModule()) RedundantDefaultArg = true; else if (!getASTContext().isSameDefaultTemplateArgument(OldTypeParm, NewTypeParm)) { @@ -2904,8 +2903,7 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams, OldDefaultLoc = OldNonTypeParm->getDefaultArgumentLoc(); NewDefaultLoc = NewNonTypeParm->getDefaultArgumentLoc(); SawDefaultArgument = true; - if (!OldNonTypeParm->getOwningModule() || - isModuleUnitOfCurrentTU(OldNonTypeParm->getOwningModule())) + if (!OldNonTypeParm->getOwningModule()) RedundantDefaultArg = true; else if (!getASTContext().isSameDefaultTemplateArgument( OldNonTypeParm, NewNonTypeParm)) { @@ -2956,8 +2954,7 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams, OldDefaultLoc = OldTemplateParm->getDefaultArgument().getLocation(); NewDefaultLoc = NewTemplateParm->getDefaultArgument().getLocation(); SawDefaultArgument = true; - if (!OldTemplateParm->getOwningModule() || - isModuleUnitOfCurrentTU(OldTemplateParm->getOwningModule())) + if (!OldTemplateParm->getOwningModule()) RedundantDefaultArg = true; else if (!getASTContext().isSameDefaultTemplateArgument( OldTemplateParm, NewTemplateParm)) { diff --git a/clang/test/Modules/redundant-template-default-arg4.cpp b/clang/test/Modules/redundant-template-default-arg4.cpp new file mode 100644 index 0000000000000..c0e1337e10abe --- /dev/null +++ b/clang/test/Modules/redundant-template-default-arg4.cpp @@ -0,0 +1,40 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -x c++ -std=c++20 -fmodules -fmodule-name=foo %t/foo.map -emit-module -o %t/foo.pcm +// RUN: %clang_cc1 -x c++ -std=c++20 -fmodules -fmodules-cache-path=%t \ +// RUN: -fmodule-file=%t/foo.pcm %t/use.cpp -verify -fsyntax-only + +//--- foo.map +module "foo" { + export * + header "foo.h" +} + +//--- foo.h +template +class A; + +template +class templ_params {}; + +template typename T2 = templ_params> +class B; + +template +class C; + +//--- use.cpp +#include "foo.h" +template // expected-error {{template parameter default argument is inconsistent with previous definition}} +class A; // expected-note@foo.h:1 {{previous default template argument defined in module foo}} + +template +class templ_params2 {}; + +template typename T2 = templ_params2> // expected-error {{template parameter default argument is inconsistent with previous definition}} +class B; // expected-note@foo.h:7 {{previous default template argument defined in module foo}} + +template // expected-error {{template parameter default argument is inconsistent with previous definition}} +class C; // expected-note@foo.h:10 {{previous default template argument defined in module foo}} diff --git a/clang/test/Modules/redundant-template-default-arg5.cpp b/clang/test/Modules/redundant-template-default-arg5.cpp new file mode 100644 index 0000000000000..5c874e03a9e51 --- /dev/null +++ b/clang/test/Modules/redundant-template-default-arg5.cpp @@ -0,0 +1,30 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 -fmodules -fmodule-name=mod -xc++ -emit-module %t/mod.cppmap -o %t/mod.pcm +// RUN: %clang_cc1 -std=c++20 -fmodules -fmodule-file=%t/mod.pcm -fsyntax-only %t/use.cc -verify + +//--- mod.cppmap +module "mod" { + export * + header "mod.h" +} + +//--- mod.h +#ifndef MOD +#define MOD +#include "templ.h" +#endif + +//--- templ.h +#ifndef TEMPL +#define TEMPL +template +inline constexpr bool inl = false; +#endif + +//--- use.cc +// expected-no-diagnostics +#include "templ.h" +#include "mod.h"