Skip to content

Commit 18162a3

Browse files
jrose-applejrose-apple
committed
Revert "AST: Simplify ModuleDecl::forAllVisibleModules()"
This reverts commit 0c32c54.
1 parent ab8d72c commit 18162a3

File tree

9 files changed

+85
-43
lines changed

9 files changed

+85
-43
lines changed

‎include/swift/AST/Module.h

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,14 +408,46 @@ class ModuleDecl : public DeclContext, public TypeDecl {
408408
///
409409
/// \param topLevelAccessPath If present, include the top-level module in the
410410
/// results, with the given access path.
411+
/// \param includePrivateTopLevelImports If true, imports listed in all
412+
/// file units within this module are traversed. Otherwise (the
413+
/// default), only re-exported imports are traversed.
411414
/// \param fn A callback of type bool(ImportedModule) or void(ImportedModule).
412415
/// Return \c false to abort iteration.
413416
///
414417
/// \return True if the traversal ran to completion, false if it ended early
415418
/// due to the callback.
416419
bool forAllVisibleModules(AccessPathTy topLevelAccessPath,
420+
bool includePrivateTopLevelImports,
417421
llvm::function_ref<bool(ImportedModule)> fn);
418422

423+
bool forAllVisibleModules(AccessPathTy topLevelAccessPath,
424+
bool includePrivateTopLevelImports,
425+
llvm::function_ref<void(ImportedModule)> fn) {
426+
return forAllVisibleModules(topLevelAccessPath,
427+
includePrivateTopLevelImports,
428+
[=](const ImportedModule &import) -> bool {
429+
fn(import);
430+
return true;
431+
});
432+
}
433+
434+
template <typename Fn>
435+
bool forAllVisibleModules(AccessPathTy topLevelAccessPath,
436+
bool includePrivateTopLevelImports,
437+
Fn &&fn) {
438+
using RetTy = typename std::result_of<Fn(ImportedModule)>::type;
439+
llvm::function_ref<RetTy(ImportedModule)> wrapped{std::forward<Fn>(fn)};
440+
return forAllVisibleModules(topLevelAccessPath,
441+
includePrivateTopLevelImports,
442+
wrapped);
443+
}
444+
445+
template <typename Fn>
446+
bool forAllVisibleModules(AccessPathTy topLevelAccessPath, Fn &&fn) {
447+
return forAllVisibleModules(topLevelAccessPath, false,
448+
std::forward<Fn>(fn));
449+
}
450+
419451
/// @}
420452

421453
using LinkLibraryCallback = llvm::function_ref<void(LinkLibrary)>;
@@ -663,6 +695,23 @@ class FileUnit : public DeclContext {
663695
bool
664696
forAllVisibleModules(llvm::function_ref<bool(ModuleDecl::ImportedModule)> fn);
665697

698+
bool
699+
forAllVisibleModules(llvm::function_ref<void(ModuleDecl::ImportedModule)> fn) {
700+
return forAllVisibleModules([=](ModuleDecl::ImportedModule import) -> bool {
701+
fn(import);
702+
return true;
703+
});
704+
}
705+
706+
template <typename Fn>
707+
bool forAllVisibleModules(Fn &&fn) {
708+
using RetTy = typename std::result_of<Fn(ModuleDecl::ImportedModule)>::type;
709+
llvm::function_ref<RetTy(ModuleDecl::ImportedModule)> wrapped{
710+
std::forward<Fn>(fn)
711+
};
712+
return forAllVisibleModules(wrapped);
713+
}
714+
666715
/// @}
667716

668717
/// True if this file contains the main class for the module.

‎lib/AST/ASTContext.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2331,7 +2331,6 @@ void ASTContext::diagnoseAttrsRequiringFoundation(SourceFile &SF) {
23312331
SF.forAllVisibleModules([&](ModuleDecl::ImportedModule import) {
23322332
if (import.second->getName() == Id_Foundation)
23332333
ImportsFoundationModule = true;
2334-
return true;
23352334
});
23362335

23372336
if (ImportsFoundationModule)

‎lib/AST/LookupVisibleDecls.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,6 @@ static void doDynamicLookup(VisibleDeclConsumer &Consumer,
387387
CurrDC->getParentSourceFile()->forAllVisibleModules(
388388
[&](ModuleDecl::ImportedModule Import) {
389389
Import.second->lookupClassMembers(Import.first, ConsumerWrapper);
390-
return true;
391390
});
392391
}
393392

‎lib/AST/Module.cpp

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,28 +1127,40 @@ bool ModuleDecl::isSystemModule() const {
11271127
return false;
11281128
}
11291129

1130+
template<bool respectVisibility, typename Callback>
11301131
static bool forAllImportedModules(ModuleDecl *topLevel,
11311132
ModuleDecl::AccessPathTy thisPath,
1132-
llvm::function_ref<bool(ModuleDecl::ImportedModule)> fn) {
1133+
bool includePrivateTopLevelImports,
1134+
const Callback &fn) {
11331135
using ImportedModule = ModuleDecl::ImportedModule;
1136+
using AccessPathTy = ModuleDecl::AccessPathTy;
11341137

11351138
llvm::SmallSet<ImportedModule, 32, ModuleDecl::OrderImportedModules> visited;
11361139
SmallVector<ImportedModule, 32> stack;
11371140

1138-
topLevel->getImportedModules(stack, ModuleDecl::ImportFilter::Public);
1141+
// Even if we're processing the top-level module like any other, we may
1142+
// still want to include non-exported modules.
1143+
ModuleDecl::ImportFilter filter = respectVisibility ? ModuleDecl::ImportFilter::Public
1144+
: ModuleDecl::ImportFilter::All;
1145+
ModuleDecl::ImportFilter topLevelFilter =
1146+
includePrivateTopLevelImports ? ModuleDecl::ImportFilter::All : filter;
1147+
topLevel->getImportedModules(stack, topLevelFilter);
11391148

11401149
// Make sure the top-level module is first; we want pre-order-ish traversal.
1141-
stack.push_back(ImportedModule(thisPath, topLevel));
1150+
AccessPathTy overridingPath;
1151+
if (respectVisibility)
1152+
overridingPath = thisPath;
1153+
stack.push_back(ImportedModule(overridingPath, topLevel));
11421154

11431155
while (!stack.empty()) {
11441156
auto next = stack.pop_back_val();
11451157

11461158
// Filter any whole-module imports, and skip specific-decl imports if the
11471159
// import path doesn't match exactly.
1148-
if (next.first.empty())
1149-
next.first = thisPath;
1150-
else if (!thisPath.empty() &&
1151-
!ModuleDecl::isSameAccessPath(next.first, thisPath)) {
1160+
if (next.first.empty() || !respectVisibility)
1161+
next.first = overridingPath;
1162+
else if (!overridingPath.empty() &&
1163+
!ModuleDecl::isSameAccessPath(next.first, overridingPath)) {
11521164
// If we ever allow importing non-top-level decls, it's possible the rule
11531165
// above isn't what we want.
11541166
assert(next.first.size() == 1 && "import of non-top-level decl");
@@ -1161,15 +1173,20 @@ static bool forAllImportedModules(ModuleDecl *topLevel,
11611173
if (!fn(next))
11621174
return false;
11631175

1164-
next.second->getImportedModulesForLookup(stack);
1176+
if (respectVisibility)
1177+
next.second->getImportedModulesForLookup(stack);
1178+
else
1179+
next.second->getImportedModules(stack, filter);
11651180
}
11661181

11671182
return true;
11681183
}
11691184

11701185
bool ModuleDecl::forAllVisibleModules(AccessPathTy thisPath,
1186+
bool includePrivateTopLevelImports,
11711187
llvm::function_ref<bool(ImportedModule)> fn) {
1172-
return forAllImportedModules(this, thisPath, fn);
1188+
return forAllImportedModules<true>(this, thisPath,
1189+
includePrivateTopLevelImports, fn);
11731190
}
11741191

11751192
bool FileUnit::forAllVisibleModules(
@@ -1198,15 +1215,13 @@ void ModuleDecl::collectLinkLibraries(LinkLibraryCallback callback) {
11981215
void
11991216
SourceFile::collectLinkLibraries(ModuleDecl::LinkLibraryCallback callback) const {
12001217

1201-
const_cast<SourceFile *>(this)->forAllVisibleModules(
1202-
[&](swift::ModuleDecl::ImportedModule import) {
1203-
swift::ModuleDecl *next = import.second;
1204-
if (next->getName() == getParentModule()->getName())
1205-
return true;
1218+
const_cast<SourceFile *>(this)->forAllVisibleModules([&](swift::ModuleDecl::ImportedModule import) {
1219+
swift::ModuleDecl *next = import.second;
1220+
if (next->getName() == getParentModule()->getName())
1221+
return;
12061222

1207-
next->collectLinkLibraries(callback);
1208-
return true;
1209-
});
1223+
next->collectLinkLibraries(callback);
1224+
});
12101225
}
12111226

12121227
bool ModuleDecl::walk(ASTWalker &Walker) {

‎lib/AST/NameLookup.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1865,7 +1865,6 @@ bool DeclContext::lookupQualified(Type type,
18651865
SmallVector<ValueDecl *, 4> allDecls;
18661866
forAllVisibleModules(this, [&](ModuleDecl::ImportedModule import) {
18671867
import.second->lookupClassMember(import.first, member, allDecls);
1868-
return true;
18691868
});
18701869

18711870
// For each declaration whose context is not something we've
@@ -1928,7 +1927,6 @@ void DeclContext::lookupAllObjCMethods(
19281927
// Collect all of the methods with this selector.
19291928
forAllVisibleModules(this, [&](ModuleDecl::ImportedModule import) {
19301929
import.second->lookupObjCMethods(selector, results);
1931-
return true;
19321930
});
19331931

19341932
// Filter out duplicates.

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,9 +1580,7 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
15801580
// FIXME: This forces the creation of wrapper modules for all imports as
15811581
// well, and may do unnecessary work.
15821582
cacheEntry.setInt(true);
1583-
result->forAllVisibleModules({}, [&](ModuleDecl::ImportedModule import) {
1584-
return true;
1585-
});
1583+
result->forAllVisibleModules({}, [&](ModuleDecl::ImportedModule import) {});
15861584
}
15871585
} else {
15881586
// Build the representation of the Clang module in Swift.
@@ -1602,9 +1600,7 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
16021600
// Force load adapter modules for all imported modules.
16031601
// FIXME: This forces the creation of wrapper modules for all imports as
16041602
// well, and may do unnecessary work.
1605-
result->forAllVisibleModules({}, [](ModuleDecl::ImportedModule import) {
1606-
return true;
1607-
});
1603+
result->forAllVisibleModules({}, [](ModuleDecl::ImportedModule import) {});
16081604
}
16091605

16101606
if (clangModule->isSubModule()) {

‎lib/IDE/CodeCompletion.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3334,8 +3334,6 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
33343334
break;
33353335
}
33363336
}
3337-
3338-
return true;
33393337
});
33403338
return results;
33413339
}
@@ -5609,11 +5607,7 @@ void CodeCompletionCallbacksImpl::doneParsing() {
56095607

56105608
// FIXME: actually check imports.
56115609
const_cast<ModuleDecl*>(Request.TheModule)
5612-
->forAllVisibleModules({},
5613-
[&](ModuleDecl::ImportedModule Import) {
5614-
handleImport(Import);
5615-
return true;
5616-
});
5610+
->forAllVisibleModules({}, handleImport);
56175611
} else {
56185612
// Add results from current module.
56195613
Lookup.getToplevelCompletions(Request.OnlyTypes);
@@ -5627,11 +5621,7 @@ void CodeCompletionCallbacksImpl::doneParsing() {
56275621
for (auto Imported : Imports) {
56285622
ModuleDecl *TheModule = Imported.second;
56295623
ModuleDecl::AccessPathTy AccessPath = Imported.first;
5630-
TheModule->forAllVisibleModules(AccessPath,
5631-
[&](ModuleDecl::ImportedModule Import) {
5632-
handleImport(Import);
5633-
return true;
5634-
});
5624+
TheModule->forAllVisibleModules(AccessPath, handleImport);
56355625
}
56365626
}
56375627
Lookup.RequestedCachedResults.reset();

‎lib/Sema/TypeChecker.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -655,8 +655,6 @@ void swift::performTypeChecking(SourceFile &SF, TopLevelContext &TLC,
655655
bindExtensionDecl(ED, TC);
656656
}
657657
}
658-
659-
return true;
660658
});
661659

662660
// Type check the top-level elements of the source file.

‎tools/swift-ide-test/swift-ide-test.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2449,8 +2449,6 @@ static int doPrintModuleImports(const CompilerInvocation &InitInvok,
24492449
llvm::outs() << " (Clang)";
24502450
llvm::outs() << "\n";
24512451
}
2452-
2453-
return true;
24542452
});
24552453
}
24562454

0 commit comments

Comments
 (0)