ChuanqiXu created this revision.
ChuanqiXu added reviewers: iains, ilya-biryukov, erichkeane.
ChuanqiXu added a project: clang-modules.
Herald added a project: All.
ChuanqiXu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Previously, when we want to compare if two module units are in the same module,
we need to compare their primary module name, which is string comparing. String
comparing is expensive and we want to avoid it. So here is the patch. This one
should be a NFC patch.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D130864
Files:
clang/include/clang/AST/ASTContext.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaLookup.cpp
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1568,38 +1568,27 @@
}
/// Determine if we could use all the declarations in the module.
-bool Sema::isUsableModule(const Module *M) {
+bool Sema::isUsableModule(const Module *M) const {
assert(M && "We shouldn't check nullness for module here");
- // Return quickly if we cached the result.
- if (UsableModuleUnitsCache.count(M))
- return true;
// If M is the global module fragment of the current translation unit. So it
// should be usable.
// [module.global.frag]p1:
// The global module fragment can be used to provide declarations that are
// attached to the global module and usable within the module unit.
- if (M == GlobalModuleFragment ||
- // If M is the module we're parsing, it should be usable. This covers the
- // private module fragment. The private module fragment is usable only if
- // it is within the current module unit. And it must be the current
- // parsing module unit if it is within the current module unit according
- // to the grammar of the private module fragment. NOTE: This is covered by
- // the following condition. The intention of the check is to avoid string
- // comparison as much as possible.
- M == getCurrentModule() ||
- // The module unit which is in the same module with the current module
- // unit is usable.
- //
- // FIXME: Here we judge if they are in the same module by comparing the
- // string. Is there any better solution?
- M->getPrimaryModuleInterfaceName() ==
- llvm::StringRef(getLangOpts().CurrentModule).split(':').first) {
- UsableModuleUnitsCache.insert(M);
- return true;
- }
-
- return false;
+ //
+ // If M is the module we're parsing, it should be usable. This covers the
+ // private module fragment. The private module fragment is usable only if
+ // it is within the current module unit. And it must be the current
+ // parsing module unit if it is within the current module unit according
+ // to the grammar of the private module fragment. NOTE: This is covered by
+ // the following condition. The intention of the check is to avoid
+ // expensive comparison as much as possible.
+ //
+ // The module unit which is in the same module with the current module
+ // unit is usable.
+ return isModuleUnitOfCurrentTU(M) ||
+ getASTContext().isInSameModule(M, getCurrentModule());
}
bool Sema::hasVisibleMergedDefinition(NamedDecl *Def) {
@@ -1609,7 +1598,7 @@
return false;
}
-bool Sema::hasMergedDefinitionInCurrentModule(NamedDecl *Def) {
+bool Sema::hasMergedDefinitionInCurrentModule(NamedDecl *Def) const {
for (const Module *Merged : Context.getModulesWithMergedDefinition(Def))
if (isUsableModule(Merged))
return true;
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1644,8 +1644,7 @@
// Partitions are part of the module, but a partition could import another
// module, so verify that the PMIs agree.
if (NewM && OldM && (NewM->isModulePartition() || OldM->isModulePartition()))
- return NewM->getPrimaryModuleInterfaceName() ==
- OldM->getPrimaryModuleInterfaceName();
+ return getASTContext().isInSameModule(NewM, OldM);
bool NewIsModuleInterface = NewM && NewM->isModulePurview();
bool OldIsModuleInterface = OldM && OldM->isModulePurview();
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -1129,7 +1129,7 @@
}
ArrayRef<Module *>
-ASTContext::getModulesWithMergedDefinition(const NamedDecl *Def) {
+ASTContext::getModulesWithMergedDefinition(const NamedDecl *Def) const {
auto MergedIt =
MergedDefModules.find(cast<NamedDecl>(Def->getCanonicalDecl()));
if (MergedIt == MergedDefModules.end())
@@ -6212,6 +6212,32 @@
llvm_unreachable("bad template name!");
}
+bool ASTContext::isInSameModule(const Module *X, const Module *Y) const {
+ // If one is not module, they must not be in the same module.
+ if (!X != !Y)
+ return false;
+
+ // If both of them are not in a module, then they must not be in the same
+ // module.
+ if (!X)
+ return false;
+
+ if (X == Y)
+ return true;
+
+ /// A header module won't be in the same module with a different address.
+ if (X->isHeaderLikeModule() || Y->isHeaderLikeModule())
+ return false;
+
+ auto getHashValueForPrimaryModule = [this](const Module *M) {
+ if (!PrimaryModuleHash.count(M))
+ PrimaryModuleHash.insert({M, llvm::hash_value(M->getPrimaryModuleInterfaceName())});
+ return PrimaryModuleHash[M];
+ };
+
+ return getHashValueForPrimaryModule(X) == getHashValueForPrimaryModule(Y);
+}
+
bool ASTContext::hasSameTemplateName(const TemplateName &X,
const TemplateName &Y) const {
return getCanonicalTemplateName(X).getAsVoidPointer() ==
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2271,10 +2271,7 @@
VisibleModuleSet VisibleModules;
- /// Cache for module units which is usable for current module.
- llvm::DenseSet<const Module *> UsableModuleUnitsCache;
-
- bool isUsableModule(const Module *M);
+ bool isUsableModule(const Module *M) const;
bool isAcceptableSlow(const NamedDecl *D, AcceptableKind Kind);
@@ -2350,7 +2347,7 @@
const NamedDecl *D, llvm::SmallVectorImpl<Module *> *Modules = nullptr);
bool hasVisibleMergedDefinition(NamedDecl *Def);
- bool hasMergedDefinitionInCurrentModule(NamedDecl *Def);
+ bool hasMergedDefinitionInCurrentModule(NamedDecl *Def) const;
/// Determine if \p D and \p Suggested have a structurally compatible
/// layout as described in C11 6.2.7/1.
Index: clang/include/clang/AST/ASTContext.h
===================================================================
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -332,6 +332,11 @@
/// data member.
mutable std::string CUIDHash;
+ /// A cache mapping modules to the hash value of their primary module name.
+ ///
+ /// This is lazily created. This is intentionally not serialized.
+ mutable llvm::DenseMap<const Module *, llvm::hash_code> PrimaryModuleHash;
+
/// Representation of a "canonical" template template parameter that
/// is used in canonical template names.
class CanonicalTemplateTemplateParm : public llvm::FoldingSetNode {
@@ -1066,7 +1071,7 @@
/// Get the additional modules in which the definition \p Def has
/// been merged.
- ArrayRef<Module*> getModulesWithMergedDefinition(const NamedDecl *Def);
+ ArrayRef<Module*> getModulesWithMergedDefinition(const NamedDecl *Def) const;
/// Add a declaration to the list of declarations that are initialized
/// for a module. This will typically be a global variable (with internal
@@ -2691,6 +2696,14 @@
bool isSameDefaultTemplateArgument(const NamedDecl *X,
const NamedDecl *Y) const;
+ /// Determine whether two module units lives in the same module.
+ /// Note that the term `Module` in implementation has different meanings
+ /// with the term in C++ spec. In C++ spec, a module could consist of
+ /// multiple module units and a module unit could consist of some of module
+ /// fragment. But `Module` in the implementation could be a clang map module,
+ /// header unit, a module unit or a module fragment.
+ bool isInSameModule(const Module *X, const Module *Y) const;
+
/// Retrieve the "canonical" template argument.
///
/// The canonical template argument is the simplest template argument
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
