ChuanqiXu updated this revision to Diff 449173.
ChuanqiXu added a comment.
Address comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130864/new/
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,31 @@
llvm_unreachable("bad template name!");
}
+bool ASTContext::isInSameModule(const Module *X, const Module *Y) const {
+ // If either one is not module unit, they must not be in the same module.
+ if (!X || !Y)
+ 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) {
+ auto Iter = PrimaryModuleHash.find(M);
+ if (Iter == PrimaryModuleHash.end()) {
+ auto HashValue = llvm::hash_value(M->getPrimaryModuleInterfaceName());
+ PrimaryModuleHash.insert({M, HashValue});
+ return HashValue;
+ }
+ return Iter->second;
+ };
+
+ 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
