Skip to content

Commit 790f931

Browse files
committed
[NFC] [Modules] Extract the logic to decide whether the module units belongs to the same module
This patch extracts the logci to decide how we decide the module units belongs to the same module into a member function of ASTContext. This is helpful to refactor the implementation in the future.
1 parent 57f7937 commit 790f931

File tree

5 files changed

+54
-34
lines changed

5 files changed

+54
-34
lines changed

clang/include/clang/AST/ASTContext.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,6 +1073,12 @@ class ASTContext : public RefCountedBase<ASTContext> {
10731073
/// Get module under construction, nullptr if this is not a C++20 module.
10741074
Module *getCurrentNamedModule() const { return CurrentCXXNamedModule; }
10751075

1076+
/// If the two module \p M1 and \p M2 are in the same module.
1077+
///
1078+
/// FIXME: The signature may be confusing since `clang::Module` means to
1079+
/// a module fragment or a module unit but not a C++20 module.
1080+
bool isInSameModule(const Module *M1, const Module *M2);
1081+
10761082
TranslationUnitDecl *getTranslationUnitDecl() const {
10771083
return TUDecl->getMostRecentDecl();
10781084
}

clang/lib/AST/ASTContext.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,15 @@ void ASTContext::setCurrentNamedModule(Module *M) {
11101110
CurrentCXXNamedModule = M;
11111111
}
11121112

1113+
bool ASTContext::isInSameModule(const Module *M1, const Module *M2) {
1114+
if (!M1 != !M2)
1115+
return false;
1116+
1117+
assert(M1 && "Shouldn't call `isInSameModule` if both M1 and M2 are none.");
1118+
return M1->getPrimaryModuleInterfaceName() ==
1119+
M2->getPrimaryModuleInterfaceName();
1120+
}
1121+
11131122
ExternCContextDecl *ASTContext::getExternCContextDecl() const {
11141123
if (!ExternCContext)
11151124
ExternCContext = ExternCContextDecl::Create(*this, getTranslationUnitDecl());

clang/lib/Sema/SemaDecl.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1663,8 +1663,7 @@ bool Sema::CheckRedeclarationModuleOwnership(NamedDecl *New, NamedDecl *Old) {
16631663
// Partitions are part of the module, but a partition could import another
16641664
// module, so verify that the PMIs agree.
16651665
if ((NewM->isModulePartition() || OldM->isModulePartition()) &&
1666-
NewM->getPrimaryModuleInterfaceName() ==
1667-
OldM->getPrimaryModuleInterfaceName())
1666+
getASTContext().isInSameModule(NewM, OldM))
16681667
return false;
16691668
}
16701669

clang/lib/Sema/SemaLookup.cpp

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,22 +1606,32 @@ bool Sema::isUsableModule(const Module *M) {
16061606
// [module.global.frag]p1:
16071607
// The global module fragment can be used to provide declarations that are
16081608
// attached to the global module and usable within the module unit.
1609-
if (M == TheGlobalModuleFragment || M == TheImplicitGlobalModuleFragment ||
1610-
// If M is the module we're parsing, it should be usable. This covers the
1611-
// private module fragment. The private module fragment is usable only if
1612-
// it is within the current module unit. And it must be the current
1613-
// parsing module unit if it is within the current module unit according
1614-
// to the grammar of the private module fragment. NOTE: This is covered by
1615-
// the following condition. The intention of the check is to avoid string
1616-
// comparison as much as possible.
1617-
M == getCurrentModule() ||
1618-
// The module unit which is in the same module with the current module
1619-
// unit is usable.
1620-
//
1621-
// FIXME: Here we judge if they are in the same module by comparing the
1622-
// string. Is there any better solution?
1623-
M->getPrimaryModuleInterfaceName() ==
1624-
llvm::StringRef(getLangOpts().CurrentModule).split(':').first) {
1609+
if (M == TheGlobalModuleFragment || M == TheImplicitGlobalModuleFragment) {
1610+
UsableModuleUnitsCache.insert(M);
1611+
return true;
1612+
}
1613+
1614+
// Otherwise, the global module fragment from other translation unit is not
1615+
// directly usable.
1616+
if (M->isGlobalModule())
1617+
return false;
1618+
1619+
Module *Current = getCurrentModule();
1620+
1621+
// If we're not parsing a module, we can't use all the declarations from
1622+
// another module easily.
1623+
if (!Current)
1624+
return false;
1625+
1626+
// If M is the module we're parsing or M and the current module unit lives in
1627+
// the same module, M should be usable.
1628+
//
1629+
// Note: It should be fine to search the vector `ModuleScopes` linearly since
1630+
// it should be generally small enough. There should be rare module fragments
1631+
// in a named module unit.
1632+
if (llvm::count_if(ModuleScopes,
1633+
[&M](const ModuleScope &MS) { return MS.Module == M; }) ||
1634+
getASTContext().isInSameModule(M, Current)) {
16251635
UsableModuleUnitsCache.insert(M);
16261636
return true;
16271637
}
@@ -5816,19 +5826,13 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, const NamedDecl *Decl,
58165826
if (M->isModuleMapModule())
58175827
return M->getFullModuleName();
58185828

5819-
Module *CurrentModule = getCurrentModule();
5820-
58215829
if (M->isImplicitGlobalModule())
58225830
M = M->getTopLevelModule();
58235831

5824-
bool IsInTheSameModule =
5825-
CurrentModule && CurrentModule->getPrimaryModuleInterfaceName() ==
5826-
M->getPrimaryModuleInterfaceName();
5827-
58285832
// If the current module unit is in the same module with M, it is OK to show
58295833
// the partition name. Otherwise, it'll be sufficient to show the primary
58305834
// module name.
5831-
if (IsInTheSameModule)
5835+
if (getASTContext().isInSameModule(M, getCurrentModule()))
58325836
return M->getTopLevelModuleName().str();
58335837
else
58345838
return M->getPrimaryModuleInterfaceName().str();

clang/lib/Sema/SemaModule.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ static std::string stringFromPath(ModuleIdPath Path) {
8282
/// CurrentModule. Since currently it is expensive to decide whether two module
8383
/// units come from the same module by comparing the module name.
8484
static bool
85-
isImportingModuleUnitFromSameModule(Module *Imported, Module *CurrentModule,
85+
isImportingModuleUnitFromSameModule(ASTContext &Ctx, Module *Imported,
86+
Module *CurrentModule,
8687
Module *&FoundPrimaryModuleInterface) {
8788
if (!Imported->isNamedModule())
8889
return false;
@@ -109,8 +110,7 @@ isImportingModuleUnitFromSameModule(Module *Imported, Module *CurrentModule,
109110
if (!CurrentModule->isModulePartitionImplementation())
110111
return false;
111112

112-
if (Imported->getPrimaryModuleInterfaceName() ==
113-
CurrentModule->getPrimaryModuleInterfaceName()) {
113+
if (Ctx.isInSameModule(Imported, CurrentModule)) {
114114
assert(!FoundPrimaryModuleInterface ||
115115
FoundPrimaryModuleInterface == Imported);
116116
FoundPrimaryModuleInterface = Imported;
@@ -127,8 +127,9 @@ isImportingModuleUnitFromSameModule(Module *Imported, Module *CurrentModule,
127127
/// the module unit purview of U. These rules can in turn lead to the
128128
/// importation of yet more translation units.
129129
static void
130-
makeTransitiveImportsVisible(VisibleModuleSet &VisibleModules, Module *Imported,
131-
Module *CurrentModule, SourceLocation ImportLoc,
130+
makeTransitiveImportsVisible(ASTContext &Ctx, VisibleModuleSet &VisibleModules,
131+
Module *Imported, Module *CurrentModule,
132+
SourceLocation ImportLoc,
132133
bool IsImportingPrimaryModuleInterface = false) {
133134
assert(Imported->isNamedModule() &&
134135
"'makeTransitiveImportsVisible()' is intended for standard C++ named "
@@ -150,7 +151,7 @@ makeTransitiveImportsVisible(VisibleModuleSet &VisibleModules, Module *Imported,
150151
// use the sourcelocation loaded from the visible modules.
151152
VisibleModules.setVisible(Importing, ImportLoc);
152153

153-
if (isImportingModuleUnitFromSameModule(Importing, CurrentModule,
154+
if (isImportingModuleUnitFromSameModule(Ctx, Importing, CurrentModule,
154155
FoundPrimaryModuleInterface))
155156
for (Module *TransImported : Importing->Imports)
156157
if (!VisibleModules.isVisible(TransImported))
@@ -484,7 +485,8 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
484485
// and return the import decl to be added to the current TU.
485486
if (Interface) {
486487

487-
makeTransitiveImportsVisible(VisibleModules, Interface, Mod, ModuleLoc,
488+
makeTransitiveImportsVisible(getASTContext(), VisibleModules, Interface,
489+
Mod, ModuleLoc,
488490
/*IsImportingPrimaryModuleInterface=*/true);
489491

490492
// Make the import decl for the interface in the impl module.
@@ -643,8 +645,8 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
643645
Diag(ImportLoc, diag::warn_experimental_header_unit);
644646

645647
if (Mod->isNamedModule())
646-
makeTransitiveImportsVisible(VisibleModules, Mod, getCurrentModule(),
647-
ImportLoc);
648+
makeTransitiveImportsVisible(getASTContext(), VisibleModules, Mod,
649+
getCurrentModule(), ImportLoc);
648650
else
649651
VisibleModules.setVisible(Mod, ImportLoc);
650652

0 commit comments

Comments
 (0)