[clang] 52bc4b1 - [NFC] [C++20] [Modules] Refactor Sema::isModuleUnitOfCurrentTU into
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Mon May 22 19:52:35 PDT 2023
Author: Chuanqi Xu
Date: 2023-05-23T10:52:22+08:00
New Revision: 52bc4b16cb68d6d64c0d9499b2e6c1d719e78085
URL: https://github.com/llvm/llvm-project/commit/52bc4b16cb68d6d64c0d9499b2e6c1d719e78085
DIFF: https://github.com/llvm/llvm-project/commit/52bc4b16cb68d6d64c0d9499b2e6c1d719e78085.diff
LOG: [NFC] [C++20] [Modules] Refactor Sema::isModuleUnitOfCurrentTU into
Decl::isInAnotherModuleUnit
Refactor `Sema::isModuleUnitOfCurrentTU` to `Decl::isInAnotherModuleUnit`
to make code simpler a little bit. Note that although this patch
introduces a FIXME, this is an existing issue and this patch just tries
to describe it explicitly.
Added:
clang/test/Modules/redundant-template-default-arg4.cpp
clang/test/Modules/redundant-template-default-arg5.cpp
Modified:
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/DeclBase.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/Sema/SemaModule.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplate.cpp
Removed:
################################################################################
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<ASTContext> {
llvm::DenseMap<Module*, PerModuleInitializers*> 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<Decl *> 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<ExportDecl>(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
- ///
diff erent 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,
diff erent entities of the same could have
- ///
diff erent 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
+ ///
diff erent 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,
diff erent entities of the same could have
+ ///
diff erent 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 T1, int V = 0>
+class A;
+
+template <typename T>
+class templ_params {};
+
+template<class T1, template <typename> typename T2 = templ_params>
+class B;
+
+template<class T1, class T2 = int>
+class C;
+
+//--- use.cpp
+#include "foo.h"
+template<class T1, int V = 1> // expected-error {{template parameter default argument is inconsistent with previous definition}}
+class A; // expected-note at foo.h:1 {{previous default template argument defined in module foo}}
+
+template <typename T>
+class templ_params2 {};
+
+template<class T1, template <typename> typename T2 = templ_params2> // expected-error {{template parameter default argument is inconsistent with previous definition}}
+class B; // expected-note at foo.h:7 {{previous default template argument defined in module foo}}
+
+template<class T1, class T2 = double> // expected-error {{template parameter default argument is inconsistent with previous definition}}
+class C; // expected-note at 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 <typename t1 = void>
+inline constexpr bool inl = false;
+#endif
+
+//--- use.cc
+// expected-no-diagnostics
+#include "templ.h"
+#include "mod.h"
More information about the cfe-commits
mailing list