Skip to content

Commit af87582

Browse files
slavapestovslavapestov
authored
Merge pull request #16206 from slavapestov/serialization-cleanup
Serialization cleanup
2 parents 58de64b + 4d7950a commit af87582

File tree

14 files changed

+113
-151
lines changed

14 files changed

+113
-151
lines changed

‎include/swift/AST/Module.h

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -400,46 +400,14 @@ class ModuleDecl : public DeclContext, public TypeDecl {
400400
///
401401
/// \param topLevelAccessPath If present, include the top-level module in the
402402
/// results, with the given access path.
403-
/// \param includePrivateTopLevelImports If true, imports listed in all
404-
/// file units within this module are traversed. Otherwise (the
405-
/// default), only re-exported imports are traversed.
406403
/// \param fn A callback of type bool(ImportedModule) or void(ImportedModule).
407404
/// Return \c false to abort iteration.
408405
///
409406
/// \return True if the traversal ran to completion, false if it ended early
410407
/// due to the callback.
411408
bool forAllVisibleModules(AccessPathTy topLevelAccessPath,
412-
bool includePrivateTopLevelImports,
413409
llvm::function_ref<bool(ImportedModule)> fn);
414410

415-
bool forAllVisibleModules(AccessPathTy topLevelAccessPath,
416-
bool includePrivateTopLevelImports,
417-
llvm::function_ref<void(ImportedModule)> fn) {
418-
return forAllVisibleModules(topLevelAccessPath,
419-
includePrivateTopLevelImports,
420-
[=](const ImportedModule &import) -> bool {
421-
fn(import);
422-
return true;
423-
});
424-
}
425-
426-
template <typename Fn>
427-
bool forAllVisibleModules(AccessPathTy topLevelAccessPath,
428-
bool includePrivateTopLevelImports,
429-
Fn &&fn) {
430-
using RetTy = typename std::result_of<Fn(ImportedModule)>::type;
431-
llvm::function_ref<RetTy(ImportedModule)> wrapped{std::forward<Fn>(fn)};
432-
return forAllVisibleModules(topLevelAccessPath,
433-
includePrivateTopLevelImports,
434-
wrapped);
435-
}
436-
437-
template <typename Fn>
438-
bool forAllVisibleModules(AccessPathTy topLevelAccessPath, Fn &&fn) {
439-
return forAllVisibleModules(topLevelAccessPath, false,
440-
std::forward<Fn>(fn));
441-
}
442-
443411
/// @}
444412

445413
using LinkLibraryCallback = llvm::function_ref<void(LinkLibrary)>;
@@ -687,23 +655,6 @@ class FileUnit : public DeclContext {
687655
bool
688656
forAllVisibleModules(llvm::function_ref<bool(ModuleDecl::ImportedModule)> fn);
689657

690-
bool
691-
forAllVisibleModules(llvm::function_ref<void(ModuleDecl::ImportedModule)> fn) {
692-
return forAllVisibleModules([=](ModuleDecl::ImportedModule import) -> bool {
693-
fn(import);
694-
return true;
695-
});
696-
}
697-
698-
template <typename Fn>
699-
bool forAllVisibleModules(Fn &&fn) {
700-
using RetTy = typename std::result_of<Fn(ModuleDecl::ImportedModule)>::type;
701-
llvm::function_ref<RetTy(ModuleDecl::ImportedModule)> wrapped{
702-
std::forward<Fn>(fn)
703-
};
704-
return forAllVisibleModules(wrapped);
705-
}
706-
707658
/// @}
708659

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

‎include/swift/Serialization/ModuleFile.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,6 @@ class ModuleFile
156156
bool isHeader() const { return IsHeader; }
157157
bool isScoped() const { return IsScoped; }
158158

159-
void forceExported() { IsExported = true; }
160-
161159
std::string getPrettyPrintedPath() const;
162160
};
163161

@@ -399,12 +397,6 @@ class ModuleFile
399397
/// Whether this module file comes from a framework.
400398
unsigned IsFramework : 1;
401399

402-
/// THIS SETTING IS OBSOLETE BUT IS STILL USED BY OLDER MODULES.
403-
///
404-
/// Whether this module has a shadowed module that's part of its public
405-
/// interface.
406-
unsigned HasUnderlyingModule : 1;
407-
408400
/// Whether or not ImportDecls is valid.
409401
unsigned ComputedImportDecls : 1;
410402

‎include/swift/Serialization/ModuleFormat.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ namespace input_block {
583583
LINK_LIBRARY,
584584
IMPORTED_HEADER,
585585
IMPORTED_HEADER_CONTENTS,
586-
MODULE_FLAGS,
586+
MODULE_FLAGS, // [unused]
587587
SEARCH_PATH
588588
};
589589

@@ -616,11 +616,6 @@ namespace input_block {
616616
BCBlob
617617
>;
618618

619-
using ModuleFlagsLayout = BCRecordLayout<
620-
MODULE_FLAGS,
621-
BCFixed<1> // has underlying module? [[UNUSED]]
622-
>;
623-
624619
using SearchPathLayout = BCRecordLayout<
625620
SEARCH_PATH,
626621
BCFixed<1>, // framework?

‎lib/AST/ASTContext.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2348,6 +2348,7 @@ void ASTContext::diagnoseAttrsRequiringFoundation(SourceFile &SF) {
23482348
SF.forAllVisibleModules([&](ModuleDecl::ImportedModule import) {
23492349
if (import.second->getName() == Id_Foundation)
23502350
ImportsFoundationModule = true;
2351+
return true;
23512352
});
23522353

23532354
if (ImportsFoundationModule)

‎lib/AST/LookupVisibleDecls.cpp

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

‎lib/AST/Module.cpp

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

1130-
template<bool respectVisibility, typename Callback>
1131-
static bool forAllImportedModules(ModuleDecl *topLevel,
1132-
ModuleDecl::AccessPathTy thisPath,
1133-
bool includePrivateTopLevelImports,
1134-
const Callback &fn) {
1135-
using ImportedModule = ModuleDecl::ImportedModule;
1136-
using AccessPathTy = ModuleDecl::AccessPathTy;
1137-
1130+
bool ModuleDecl::forAllVisibleModules(AccessPathTy thisPath,
1131+
llvm::function_ref<bool(ImportedModule)> fn) {
11381132
llvm::SmallSet<ImportedModule, 32, ModuleDecl::OrderImportedModules> visited;
11391133
SmallVector<ImportedModule, 32> stack;
11401134

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);
1135+
getImportedModules(stack, ModuleDecl::ImportFilter::Public);
11481136

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

11551140
while (!stack.empty()) {
11561141
auto next = stack.pop_back_val();
11571142

11581143
// Filter any whole-module imports, and skip specific-decl imports if the
11591144
// import path doesn't match exactly.
1160-
if (next.first.empty() || !respectVisibility)
1161-
next.first = overridingPath;
1162-
else if (!overridingPath.empty() &&
1163-
!ModuleDecl::isSameAccessPath(next.first, overridingPath)) {
1145+
if (next.first.empty())
1146+
next.first = thisPath;
1147+
else if (!thisPath.empty() &&
1148+
!ModuleDecl::isSameAccessPath(next.first, thisPath)) {
11641149
// If we ever allow importing non-top-level decls, it's possible the rule
11651150
// above isn't what we want.
11661151
assert(next.first.size() == 1 && "import of non-top-level decl");
@@ -1173,22 +1158,12 @@ static bool forAllImportedModules(ModuleDecl *topLevel,
11731158
if (!fn(next))
11741159
return false;
11751160

1176-
if (respectVisibility)
1177-
next.second->getImportedModulesForLookup(stack);
1178-
else
1179-
next.second->getImportedModules(stack, filter);
1161+
next.second->getImportedModulesForLookup(stack);
11801162
}
11811163

11821164
return true;
11831165
}
11841166

1185-
bool ModuleDecl::forAllVisibleModules(AccessPathTy thisPath,
1186-
bool includePrivateTopLevelImports,
1187-
llvm::function_ref<bool(ImportedModule)> fn) {
1188-
return forAllImportedModules<true>(this, thisPath,
1189-
includePrivateTopLevelImports, fn);
1190-
}
1191-
11921167
bool FileUnit::forAllVisibleModules(
11931168
llvm::function_ref<bool(ModuleDecl::ImportedModule)> fn) {
11941169
if (!getParentModule()->forAllVisibleModules(ModuleDecl::AccessPathTy(), fn))
@@ -1215,13 +1190,15 @@ void ModuleDecl::collectLinkLibraries(LinkLibraryCallback callback) {
12151190
void
12161191
SourceFile::collectLinkLibraries(ModuleDecl::LinkLibraryCallback callback) const {
12171192

1218-
const_cast<SourceFile *>(this)->forAllVisibleModules([&](swift::ModuleDecl::ImportedModule import) {
1219-
swift::ModuleDecl *next = import.second;
1220-
if (next->getName() == getParentModule()->getName())
1221-
return;
1193+
const_cast<SourceFile *>(this)->forAllVisibleModules(
1194+
[&](swift::ModuleDecl::ImportedModule import) {
1195+
swift::ModuleDecl *next = import.second;
1196+
if (next->getName() == getParentModule()->getName())
1197+
return true;
12221198

1223-
next->collectLinkLibraries(callback);
1224-
});
1199+
next->collectLinkLibraries(callback);
1200+
return true;
1201+
});
12251202
}
12261203

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

‎lib/AST/NameLookup.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1865,6 +1865,7 @@ 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;
18681869
});
18691870

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

19321934
// Filter out duplicates.

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 54 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,7 +1576,9 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
15761576
// FIXME: This forces the creation of wrapper modules for all imports as
15771577
// well, and may do unnecessary work.
15781578
cacheEntry.setInt(true);
1579-
result->forAllVisibleModules({}, [&](ModuleDecl::ImportedModule import) {});
1579+
result->forAllVisibleModules({}, [&](ModuleDecl::ImportedModule import) {
1580+
return true;
1581+
});
15801582
}
15811583
} else {
15821584
// Build the representation of the Clang module in Swift.
@@ -1595,7 +1597,9 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
15951597
// Force load adapter modules for all imported modules.
15961598
// FIXME: This forces the creation of wrapper modules for all imports as
15971599
// well, and may do unnecessary work.
1598-
result->forAllVisibleModules({}, [](ModuleDecl::ImportedModule import) {});
1600+
result->forAllVisibleModules({}, [](ModuleDecl::ImportedModule import) {
1601+
return true;
1602+
});
15991603
}
16001604

16011605
if (clangModule->isSubModule()) {
@@ -3039,43 +3043,70 @@ ModuleDecl *ClangModuleUnit::getAdapterModule() const {
30393043
void ClangModuleUnit::getImportedModules(
30403044
SmallVectorImpl<ModuleDecl::ImportedModule> &imports,
30413045
ModuleDecl::ImportFilter filter) const {
3042-
if (filter != ModuleDecl::ImportFilter::Public)
3046+
switch (filter) {
3047+
case ModuleDecl::ImportFilter::All:
3048+
case ModuleDecl::ImportFilter::Private:
30433049
imports.push_back({ModuleDecl::AccessPathTy(), owner.getStdlibModule()});
3050+
break;
3051+
case ModuleDecl::ImportFilter::Public:
3052+
break;
3053+
}
30443054

30453055
SmallVector<clang::Module *, 8> imported;
30463056
if (!clangModule) {
30473057
// This is the special "imported headers" module.
3048-
if (filter != ModuleDecl::ImportFilter::Private) {
3058+
switch (filter) {
3059+
case ModuleDecl::ImportFilter::All:
3060+
case ModuleDecl::ImportFilter::Public:
30493061
imported.append(owner.ImportedHeaderExports.begin(),
30503062
owner.ImportedHeaderExports.end());
3063+
break;
3064+
3065+
case ModuleDecl::ImportFilter::Private:
3066+
break;
30513067
}
30523068

30533069
} else {
30543070
clangModule->getExportedModules(imported);
3055-
if (filter != ModuleDecl::ImportFilter::Public) {
3056-
if (filter == ModuleDecl::ImportFilter::All) {
3057-
llvm::SmallPtrSet<clang::Module *, 8> knownModules;
3058-
imported.append(clangModule->Imports.begin(), clangModule->Imports.end());
3059-
imported.erase(std::remove_if(imported.begin(), imported.end(),
3060-
[&](clang::Module *mod) -> bool {
3061-
return !knownModules.insert(mod).second;
3062-
}),
3063-
imported.end());
3064-
} else {
3065-
llvm::SmallPtrSet<clang::Module *, 8> knownModules(imported.begin(),
3066-
imported.end());
3067-
SmallVector<clang::Module *, 8> privateImports;
3068-
std::copy_if(clangModule->Imports.begin(), clangModule->Imports.end(),
3069-
std::back_inserter(privateImports), [&](clang::Module *mod) {
3070-
return knownModules.count(mod) == 0;
3071-
});
3072-
imported.swap(privateImports);
3073-
}
3071+
3072+
switch (filter) {
3073+
case ModuleDecl::ImportFilter::All: {
3074+
llvm::SmallPtrSet<clang::Module *, 8> knownModules;
3075+
imported.append(clangModule->Imports.begin(), clangModule->Imports.end());
3076+
imported.erase(std::remove_if(imported.begin(), imported.end(),
3077+
[&](clang::Module *mod) -> bool {
3078+
return !knownModules.insert(mod).second;
3079+
}),
3080+
imported.end());
30743081

30753082
// FIXME: The parent module isn't exactly a private import, but it is
30763083
// needed for link dependencies.
30773084
if (clangModule->Parent)
30783085
imported.push_back(clangModule->Parent);
3086+
3087+
break;
3088+
}
3089+
3090+
case ModuleDecl::ImportFilter::Private: {
3091+
llvm::SmallPtrSet<clang::Module *, 8> knownModules(imported.begin(),
3092+
imported.end());
3093+
SmallVector<clang::Module *, 8> privateImports;
3094+
std::copy_if(clangModule->Imports.begin(), clangModule->Imports.end(),
3095+
std::back_inserter(privateImports), [&](clang::Module *mod) {
3096+
return knownModules.count(mod) == 0;
3097+
});
3098+
imported.swap(privateImports);
3099+
3100+
// FIXME: The parent module isn't exactly a private import, but it is
3101+
// needed for link dependencies.
3102+
if (clangModule->Parent)
3103+
imported.push_back(clangModule->Parent);
3104+
3105+
break;
3106+
}
3107+
3108+
case ModuleDecl::ImportFilter::Public:
3109+
break;
30793110
}
30803111
}
30813112

‎lib/IDE/CodeCompletion.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3329,6 +3329,8 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
33293329
break;
33303330
}
33313331
}
3332+
3333+
return true;
33323334
});
33333335
return results;
33343336
}
@@ -5602,7 +5604,11 @@ void CodeCompletionCallbacksImpl::doneParsing() {
56025604

56035605
// FIXME: actually check imports.
56045606
const_cast<ModuleDecl*>(Request.TheModule)
5605-
->forAllVisibleModules({}, handleImport);
5607+
->forAllVisibleModules({},
5608+
[&](ModuleDecl::ImportedModule Import) {
5609+
handleImport(Import);
5610+
return true;
5611+
});
56065612
} else {
56075613
// Add results from current module.
56085614
Lookup.getToplevelCompletions(Request.OnlyTypes);
@@ -5616,7 +5622,11 @@ void CodeCompletionCallbacksImpl::doneParsing() {
56165622
for (auto Imported : Imports) {
56175623
ModuleDecl *TheModule = Imported.second;
56185624
ModuleDecl::AccessPathTy AccessPath = Imported.first;
5619-
TheModule->forAllVisibleModules(AccessPath, handleImport);
5625+
TheModule->forAllVisibleModules(AccessPath,
5626+
[&](ModuleDecl::ImportedModule Import) {
5627+
handleImport(Import);
5628+
return true;
5629+
});
56205630
}
56215631
}
56225632
Lookup.RequestedCachedResults.reset();

0 commit comments

Comments
 (0)