Skip to content

Commit e2f50b5

Browse files
artemcmartemcm
committed
[Dependency Scanning] Miscellaneous fixes to import statement info serialization
- Deserialization of binary module dependencies was still relying on stale code (e.g. 'currentModuleImports', 'currentOptionalModuleImports') from serialized import strings, instead of the now in-use import infos. - Imports without a source location (e.g. implicit imports of stdlib) were not getting serialized at all - Optional import arrays were not being written out at all
1 parent ce680ea commit e2f50b5

File tree

2 files changed

+116
-94
lines changed

2 files changed

+116
-94
lines changed

‎lib/DependencyScan/ModuleDependencyCacheSerialization.cpp‎

Lines changed: 87 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -218,9 +218,6 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
218218

219219
bool hasCurrentModule = false;
220220
std::string currentModuleName;
221-
std::vector<ScannerImportStatementInfo> currentModuleImports;
222-
std::vector<ScannerImportStatementInfo> currentOptionalModuleImports;
223-
224221
std::vector<ModuleDependencyID> importedSwiftDependenciesIDs;
225222
std::vector<ModuleDependencyID> importedClangDependenciesIDs;
226223
std::vector<ModuleDependencyID> crossImportOverlayDependenciesIDs;
@@ -235,18 +232,8 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
235232
std::vector<std::string> auxiliaryFiles;
236233

237234
auto addCommonDependencyInfo =
238-
[&currentModuleImports, &currentOptionalModuleImports,
239-
&importedClangDependenciesIDs, &auxiliaryFiles,
235+
[&importedClangDependenciesIDs, &auxiliaryFiles,
240236
&macroDependencies](ModuleDependencyInfo &moduleDep) {
241-
// Add imports of this module
242-
for (const auto &moduleName : currentModuleImports)
243-
moduleDep.addModuleImport(moduleName.importIdentifier,
244-
moduleName.isExported);
245-
// Add optional imports of this module
246-
for (const auto &moduleName : currentOptionalModuleImports)
247-
moduleDep.addOptionalModuleImport(moduleName.importIdentifier,
248-
moduleName.isExported);
249-
250237
// Add qualified dependencies of this module
251238
moduleDep.setImportedClangDependencies(importedClangDependenciesIDs);
252239

@@ -297,7 +284,7 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
297284
llvm::report_fatal_error("Bad bridging module dependencies");
298285
llvm::StringSet<> alreadyAdded;
299286
std::vector<ModuleDependencyID> bridgingModuleDepIDs;
300-
for (const auto &mod : bridgingModuleDeps.value())
287+
for (const auto &mod : *bridgingModuleDeps)
301288
bridgingModuleDepIDs.push_back(
302289
ModuleDependencyID{mod, ModuleDependencyKind::Clang});
303290
moduleDep.setHeaderClangDependencies(bridgingModuleDepIDs);
@@ -364,7 +351,7 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
364351
llvm::report_fatal_error("Bad link library identifier");
365352

366353
LinkLibraries.emplace_back(
367-
libraryIdentifier.value(),
354+
*libraryIdentifier,
368355
isFramework ? LibraryKind::Framework
369356
: LibraryKind::Library,
370357
isStatic, shouldForceLoad);
@@ -395,8 +382,8 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
395382
llvm::report_fatal_error("Bad macro dependency: no executable path");
396383

397384
MacroDependencies.push_back(
398-
{macroModuleName.value(),
399-
MacroPluginDependency{libraryPath.value(), executablePath.value()}});
385+
{*macroModuleName,
386+
MacroPluginDependency{*libraryPath, *executablePath}});
400387
break;
401388
}
402389

@@ -422,10 +409,14 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
422409
if (!bufferIdentifier)
423410
llvm::report_fatal_error(
424411
"Bad import statement info: no buffer identifier");
425-
ImportStatements.push_back(ScannerImportStatementInfo(
426-
importIdentifier.value(), isExported,
427-
ScannerImportStatementInfo::ImportDiagnosticLocationInfo(
428-
bufferIdentifier.value(), lineNumber, columnNumber)));
412+
if (bufferIdentifier->empty())
413+
ImportStatements.push_back(ScannerImportStatementInfo(
414+
*importIdentifier, isExported));
415+
else
416+
ImportStatements.push_back(ScannerImportStatementInfo(
417+
*importIdentifier, isExported,
418+
ScannerImportStatementInfo::ImportDiagnosticLocationInfo(
419+
*bufferIdentifier, lineNumber, columnNumber)));
429420
break;
430421
}
431422

@@ -470,18 +461,16 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
470461
getImportStatementInfoArray(moduleImportsArrayID);
471462
if (!optionalImportStatementInfos)
472463
llvm::report_fatal_error("Bad direct Swift dependencies: no imports");
473-
importStatements = optionalImportStatementInfos.value();
464+
importStatements = *optionalImportStatementInfos;
474465

475466
auto optionalOptionalImportStatementInfos =
476467
getOptionalImportStatementInfoArray(optionalImportsArrayID);
477-
if (!optionalOptionalImportStatementInfos)
478-
llvm::report_fatal_error(
479-
"Bad direct Swift dependencies: no optional imports");
480-
optionalImportStatements = optionalOptionalImportStatementInfos.value();
468+
if (optionalOptionalImportStatementInfos)
469+
optionalImportStatements = *optionalOptionalImportStatementInfos;
481470

482471
auto optionalAuxiliaryFiles = getStringArray(AuxiliaryFilesArrayID);
483-
if (optionalAuxiliaryFiles.has_value())
484-
for (const auto &af : optionalAuxiliaryFiles.value())
472+
if (optionalAuxiliaryFiles)
473+
for (const auto &af : *optionalAuxiliaryFiles)
485474
auxiliaryFiles.push_back(af);
486475

487476
auto optionalImportedSwiftDependenciesIDs =
@@ -490,30 +479,30 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
490479
llvm::report_fatal_error(
491480
"Bad direct Swift dependencies: no qualified dependencies");
492481
importedSwiftDependenciesIDs =
493-
optionalImportedSwiftDependenciesIDs.value();
482+
*optionalImportedSwiftDependenciesIDs;
494483

495484
auto optionalImportedClangDependenciesIDs =
496485
getModuleDependencyIDArray(importedClangDependenciesIDsArrayID);
497486
if (!optionalImportedClangDependenciesIDs)
498487
llvm::report_fatal_error(
499488
"Bad direct Clang dependencies: no qualified dependencies");
500489
importedClangDependenciesIDs =
501-
optionalImportedClangDependenciesIDs.value();
490+
*optionalImportedClangDependenciesIDs;
502491

503492
auto optionalCrossImportOverlayDependenciesIDs =
504493
getModuleDependencyIDArray(crossImportOverlayDependenciesIDsArrayID);
505494
if (!optionalCrossImportOverlayDependenciesIDs)
506495
llvm::report_fatal_error(
507496
"Bad Cross-Import Overlay dependencies: no qualified dependencies");
508497
crossImportOverlayDependenciesIDs =
509-
optionalCrossImportOverlayDependenciesIDs.value();
498+
*optionalCrossImportOverlayDependenciesIDs;
510499

511500
auto optionalSwiftOverlayDependenciesIDs =
512501
getModuleDependencyIDArray(swiftOverlayDependenciesIDsArrayID);
513502
if (!optionalSwiftOverlayDependenciesIDs)
514503
llvm::report_fatal_error(
515504
"Bad Swift Overlay dependencies: no qualified dependencies");
516-
swiftOverlayDependenciesIDs = optionalSwiftOverlayDependenciesIDs.value();
505+
swiftOverlayDependenciesIDs = *optionalSwiftOverlayDependenciesIDs;
517506

518507
auto optionalLinkLibraries = getLinkLibraryArray(linkLibraryArrayID);
519508
if (!optionalLinkLibraries)
@@ -587,7 +576,7 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
587576

588577
// Form the dependencies storage object
589578
auto moduleDep = ModuleDependencyInfo::forSwiftInterfaceModule(
590-
optionalSwiftInterfaceFile.value(), compiledCandidatesRefs,
579+
*optionalSwiftInterfaceFile, compiledCandidatesRefs,
591580
buildCommandRefs, importStatements, optionalImportStatements,
592581
linkLibraries, isFramework, isStatic, *rootFileSystemID,
593582
*moduleCacheKey, *userModuleVersion);
@@ -711,7 +700,7 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
711700
// Form the dependencies storage object
712701
auto moduleDep = ModuleDependencyInfo::forSwiftBinaryModule(
713702
*compiledModulePath, *moduleDocPath, *moduleSourceInfoPath,
714-
currentModuleImports, currentOptionalModuleImports, linkLibraries,
703+
importStatements, optionalImportStatements, linkLibraries,
715704
*headerImport, *definingInterfacePath, isFramework, isStatic,
716705
*moduleCacheKey, *userModuleVersion);
717706

@@ -984,7 +973,7 @@ ModuleDependenciesCacheDeserializer::getOptionalImportStatementInfoArray(
984973
std::optional<std::vector<ModuleDependencyID>>
985974
ModuleDependenciesCacheDeserializer::getModuleDependencyIDArray(unsigned n) {
986975
auto encodedIdentifierStringArray = getStringArray(n);
987-
if (encodedIdentifierStringArray.has_value()) {
976+
if (encodedIdentifierStringArray) {
988977
static const std::string textualPrefix("swiftTextual");
989978
static const std::string binaryPrefix("swiftBinary");
990979
static const std::string placeholderPrefix("swiftPlaceholder");
@@ -1288,8 +1277,8 @@ void ModuleDependenciesCacheSerializer::writeLinkLibraries(
12881277
for (const auto &entry : modMap) {
12891278
ModuleDependencyID moduleID = {entry.getKey().str(), kind};
12901279
auto optionalDependencyInfo = cache.findDependency(moduleID);
1291-
assert(optionalDependencyInfo.has_value() && "Expected dependency info.");
1292-
auto dependencyInfo = optionalDependencyInfo.value();
1280+
assert(optionalDependencyInfo && "Expected dependency info.");
1281+
auto dependencyInfo = *optionalDependencyInfo;
12931282
unsigned numLLs = writeLinkLibraryInfos(*dependencyInfo);
12941283
moduleLLArrayMap.insert({moduleID, std::make_pair(lastLLIndex, numLLs)});
12951284
lastLLIndex += numLLs;
@@ -1343,8 +1332,8 @@ void ModuleDependenciesCacheSerializer::writeMacroDependencies(
13431332
for (const auto &entry : modMap) {
13441333
ModuleDependencyID moduleID = {entry.getKey().str(), kind};
13451334
auto optionalDependencyInfo = cache.findDependency(moduleID);
1346-
assert(optionalDependencyInfo.has_value() && "Expected dependency info.");
1347-
auto dependencyInfo = optionalDependencyInfo.value();
1335+
assert(optionalDependencyInfo && "Expected dependency info.");
1336+
auto dependencyInfo = *optionalDependencyInfo;
13481337
unsigned numMDs = writeMacroDependencies(*dependencyInfo);
13491338
moduleMacroDepArrayMap.insert(
13501339
{moduleID, std::make_pair(lastMDIndex, numMDs)});
@@ -1399,11 +1388,11 @@ void ModuleDependenciesCacheSerializer::writeImportStatementInfos(
13991388
for (auto kind = ModuleDependencyKind::FirstKind;
14001389
kind != ModuleDependencyKind::LastKind; ++kind) {
14011390
auto modMap = cache.getDependenciesMap(kind);
1402-
for (const auto &entry : modMap) {
1403-
ModuleDependencyID moduleID = {entry.getKey().str(), kind};
1391+
for (const auto &entry : modMap.keys()) {
1392+
ModuleDependencyID moduleID = {entry.str(), kind};
14041393
auto optionalDependencyInfo = cache.findDependency(moduleID);
1405-
assert(optionalDependencyInfo.has_value() && "Expected dependency info.");
1406-
auto dependencyInfo = optionalDependencyInfo.value();
1394+
assert(optionalDependencyInfo && "Expected dependency info.");
1395+
auto dependencyInfo = *optionalDependencyInfo;
14071396

14081397
auto numImportInfos =
14091398
writeImportStatementInfos(*dependencyInfo, /* optional */ false);
@@ -1414,22 +1403,28 @@ void ModuleDependenciesCacheSerializer::writeImportStatementInfos(
14141403
auto numOptionalImportInfos =
14151404
writeImportStatementInfos(*dependencyInfo, /* optional */ true);
14161405
optionalImportInfoArrayMap.insert(
1417-
{moduleID, std::make_pair(lastImportInfoIndex, numImportInfos)});
1406+
{moduleID, std::make_pair(lastImportInfoIndex, numOptionalImportInfos)});
14181407
lastImportInfoIndex += numOptionalImportInfos;
14191408
}
14201409
}
14211410

14221411
unsigned lastImportInfoArrayIndex = 1;
1412+
unsigned lastOptionalImportInfoArrayIndex = 1;
14231413
for (auto kind = ModuleDependencyKind::FirstKind;
14241414
kind != ModuleDependencyKind::LastKind; ++kind) {
14251415
auto modMap = cache.getDependenciesMap(kind);
1426-
for (const auto &entry : modMap) {
1427-
ModuleDependencyID moduleID = {entry.getKey().str(), kind};
1416+
for (const auto &entry : modMap.keys()) {
1417+
ModuleDependencyID moduleID = {entry.str(), kind};
14281418
auto entries = importInfoArrayMap.at(moduleID);
1429-
if (entries.second == 0)
1430-
continue;
1431-
writeImportStatementInfosArray(entries.first, entries.second);
1432-
ImportInfosArrayIDsMap.insert({moduleID, lastImportInfoArrayIndex++});
1419+
if (entries.second != 0) {
1420+
writeImportStatementInfosArray(entries.first, entries.second);
1421+
ImportInfosArrayIDsMap.insert({moduleID, lastImportInfoArrayIndex++});
1422+
}
1423+
auto optionalEntries = optionalImportInfoArrayMap.at(moduleID);
1424+
if (optionalEntries.second != 0) {
1425+
writeImportStatementInfosArray(optionalEntries.first, optionalEntries.second);
1426+
OptionalImportInfosArrayIDsMap.insert({moduleID, lastOptionalImportInfoArrayIndex++});
1427+
}
14331428
}
14341429
}
14351430
}
@@ -1440,22 +1435,31 @@ unsigned ModuleDependenciesCacheSerializer::writeImportStatementInfos(
14401435
size_t count = 0;
14411436
auto emitImportStatementInfo = [this, &count](const auto &importInfo,
14421437
bool isOptional) {
1443-
for (auto &importLoc : importInfo.importLocations) {
1438+
if (importInfo.importLocations.empty()) {
14441439
ImportStatementLayout::emitRecord(
14451440
Out, ScratchRecord, AbbrCodes[ImportStatementLayout::Code],
14461441
getIdentifier(importInfo.importIdentifier),
1447-
getIdentifier(importLoc.bufferIdentifier), importLoc.lineNumber,
1448-
importLoc.columnNumber, isOptional, importInfo.isExported);
1442+
0, 0, 0, isOptional, importInfo.isExported);
14491443
count++;
1444+
} else {
1445+
for (auto &importLoc : importInfo.importLocations) {
1446+
ImportStatementLayout::emitRecord(
1447+
Out, ScratchRecord, AbbrCodes[ImportStatementLayout::Code],
1448+
getIdentifier(importInfo.importIdentifier),
1449+
getIdentifier(importLoc.bufferIdentifier), importLoc.lineNumber,
1450+
importLoc.columnNumber, isOptional, importInfo.isExported);
1451+
count++;
1452+
}
14501453
}
14511454
};
14521455

1453-
for (auto &importInfo : dependencyInfo.getModuleImports())
1454-
emitImportStatementInfo(importInfo, false);
1455-
1456-
for (auto &importInfo : dependencyInfo.getOptionalModuleImports())
1457-
emitImportStatementInfo(importInfo, true);
1458-
1456+
if (!optional) {
1457+
for (auto &importInfo : dependencyInfo.getModuleImports())
1458+
emitImportStatementInfo(importInfo, false);
1459+
} else {
1460+
for (auto &importInfo : dependencyInfo.getOptionalModuleImports())
1461+
emitImportStatementInfo(importInfo, true);
1462+
}
14591463
return count;
14601464
}
14611465

@@ -1500,8 +1504,8 @@ void ModuleDependenciesCacheSerializer::writeModuleInfo(
15001504
getIdentifier(swiftTextDeps->swiftInterfaceFile);
15011505
unsigned bridgingHeaderFileId =
15021506
swiftTextDeps->textualModuleDetails.bridgingHeaderFile
1503-
? getIdentifier(swiftTextDeps->textualModuleDetails
1504-
.bridgingHeaderFile.value())
1507+
? getIdentifier(*(swiftTextDeps->textualModuleDetails
1508+
.bridgingHeaderFile))
15051509
: 0;
15061510
SwiftInterfaceModuleDetailsLayout::emitRecord(
15071511
Out, ScratchRecord, AbbrCodes[SwiftInterfaceModuleDetailsLayout::Code],
@@ -1529,8 +1533,8 @@ void ModuleDependenciesCacheSerializer::writeModuleInfo(
15291533
assert(swiftSourceDeps);
15301534
unsigned bridgingHeaderFileId =
15311535
swiftSourceDeps->textualModuleDetails.bridgingHeaderFile
1532-
? getIdentifier(swiftSourceDeps->textualModuleDetails
1533-
.bridgingHeaderFile.value())
1536+
? getIdentifier(*(swiftSourceDeps->textualModuleDetails
1537+
.bridgingHeaderFile))
15341538
: 0;
15351539
SwiftSourceModuleDetailsLayout::emitRecord(
15361540
Out, ScratchRecord, AbbrCodes[SwiftSourceModuleDetailsLayout::Code],
@@ -1732,20 +1736,14 @@ void ModuleDependenciesCacheSerializer::collectStringsAndArrays(
17321736
for (auto kind = ModuleDependencyKind::FirstKind;
17331737
kind != ModuleDependencyKind::LastKind; ++kind) {
17341738
auto modMap = cache.getDependenciesMap(kind);
1735-
for (const auto &entry : modMap) {
1736-
ModuleDependencyID moduleID = {entry.getKey().str(), kind};
1739+
for (const auto &entry : modMap.keys()) {
1740+
ModuleDependencyID moduleID = {entry.str(), kind};
17371741
auto optionalDependencyInfo = cache.findDependency(moduleID);
1738-
assert(optionalDependencyInfo.has_value() && "Expected dependency info.");
1739-
auto dependencyInfo = optionalDependencyInfo.value();
1742+
assert(optionalDependencyInfo && "Expected dependency info.");
1743+
auto dependencyInfo = *optionalDependencyInfo;
17401744
// Add the module's name
17411745
addIdentifier(moduleID.ModuleName);
17421746

1743-
// Map import infos to their respective module identifiers
1744-
auto importInfoArrayToIdentifier =
1745-
[](const auto &importInfo) -> std::string {
1746-
return importInfo.importIdentifier;
1747-
};
1748-
17491747
for (const auto &ll : dependencyInfo->getLinkLibraries())
17501748
addIdentifier(ll.getName().str());
17511749

@@ -1754,22 +1752,18 @@ void ModuleDependenciesCacheSerializer::collectStringsAndArrays(
17541752
addIdentifier(md.second.LibraryPath);
17551753
addIdentifier(md.second.ExecutablePath);
17561754
}
1755+
1756+
for (const auto &ii : dependencyInfo->getModuleImports()) {
1757+
addIdentifier(ii.importIdentifier);
1758+
for (const auto &il : ii.importLocations)
1759+
addIdentifier(il.bufferIdentifier);
1760+
}
17571761

1758-
// Add the module's imports
1759-
std::vector<std::string> importIdentifiers;
1760-
llvm::transform(dependencyInfo->getModuleImports(),
1761-
std::back_inserter(importIdentifiers),
1762-
importInfoArrayToIdentifier);
1763-
std::vector<std::string> optionalImportIdentifiers;
1764-
llvm::transform(dependencyInfo->getOptionalModuleImports(),
1765-
std::back_inserter(optionalImportIdentifiers),
1766-
importInfoArrayToIdentifier);
1767-
1768-
addStringArray(moduleID, ModuleIdentifierArrayKind::DependencyImports,
1769-
importIdentifiers);
1770-
addStringArray(moduleID,
1771-
ModuleIdentifierArrayKind::OptionalDependencyImports,
1772-
optionalImportIdentifiers);
1762+
for (const auto &oii : dependencyInfo->getOptionalModuleImports()) {
1763+
addIdentifier(oii.importIdentifier);
1764+
for (const auto &oil : oii.importLocations)
1765+
addIdentifier(oil.bufferIdentifier);
1766+
}
17731767

17741768
addDependencyIDArray(
17751769
moduleID, ModuleIdentifierArrayKind::ImportedSwiftDependenciesIDs,
@@ -1806,9 +1800,9 @@ void ModuleDependenciesCacheSerializer::collectStringsAndArrays(
18061800
addStringArray(moduleID, ModuleIdentifierArrayKind::BuildCommandLine,
18071801
swiftTextDeps->textualModuleDetails.buildCommandLine);
18081802
addIdentifier(swiftTextDeps->contextHash);
1809-
if (swiftTextDeps->textualModuleDetails.bridgingHeaderFile.has_value())
1803+
if (swiftTextDeps->textualModuleDetails.bridgingHeaderFile)
18101804
addIdentifier(
1811-
swiftTextDeps->textualModuleDetails.bridgingHeaderFile.value());
1805+
*(swiftTextDeps->textualModuleDetails.bridgingHeaderFile));
18121806
addStringArray(moduleID, ModuleIdentifierArrayKind::SourceFiles,
18131807
std::vector<std::string>());
18141808
addStringArray(moduleID, ModuleIdentifierArrayKind::BridgingSourceFiles,
@@ -1855,10 +1849,9 @@ void ModuleDependenciesCacheSerializer::collectStringsAndArrays(
18551849
case swift::ModuleDependencyKind::SwiftSource: {
18561850
auto swiftSourceDeps = dependencyInfo->getAsSwiftSourceModule();
18571851
assert(swiftSourceDeps);
1858-
if (swiftSourceDeps->textualModuleDetails.bridgingHeaderFile
1859-
.has_value())
1852+
if (swiftSourceDeps->textualModuleDetails.bridgingHeaderFile)
18601853
addIdentifier(
1861-
swiftSourceDeps->textualModuleDetails.bridgingHeaderFile.value());
1854+
*(swiftSourceDeps->textualModuleDetails.bridgingHeaderFile));
18621855
addStringArray(moduleID, ModuleIdentifierArrayKind::SourceFiles,
18631856
swiftSourceDeps->sourceFiles);
18641857
addStringArray(

0 commit comments

Comments
 (0)