Skip to content

Commit 78153f1

Browse files
artemcmartemcm
authored
Merge pull request #79676 from artemcm/RefineCrossImportOverlayResolutionLogic
[Dependency Scanning] Refine cross-import overlay detection algorithm
2 parents 62dc762 + 0555764 commit 78153f1

14 files changed

+129
-63
lines changed

‎include/swift/AST/ModuleDependencies.h

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,14 @@ struct ScannerImportStatementInfo {
154154
uint32_t columnNumber;
155155
};
156156

157-
ScannerImportStatementInfo(std::string importIdentifier)
158-
: importLocations(), importIdentifier(importIdentifier) {}
157+
ScannerImportStatementInfo(std::string importIdentifier, bool isExported)
158+
: importLocations(), importIdentifier(importIdentifier),
159+
isExported(isExported) {}
159160

160-
ScannerImportStatementInfo(std::string importIdentifier,
161+
ScannerImportStatementInfo(std::string importIdentifier, bool isExported,
161162
ImportDiagnosticLocationInfo location)
162-
: importLocations({location}), importIdentifier(importIdentifier) {}
163+
: importLocations({location}), importIdentifier(importIdentifier),
164+
isExported(isExported) {}
163165

164166
void addImportLocation(ImportDiagnosticLocationInfo location) {
165167
importLocations.push_back(location);
@@ -169,6 +171,8 @@ struct ScannerImportStatementInfo {
169171
SmallVector<ImportDiagnosticLocationInfo, 4> importLocations;
170172
/// Imported module string. e.g. "Foo.Bar" in 'import Foo.Bar'
171173
std::string importIdentifier;
174+
/// Is this an @_exported import
175+
bool isExported;
172176
};
173177

174178
/// Base class for the variant storage of ModuleDependencyInfo.
@@ -927,7 +931,7 @@ class ModuleDependencyInfo {
927931

928932
/// Add a dependency on the given module, if it was not already in the set.
929933
void
930-
addOptionalModuleImport(StringRef module,
934+
addOptionalModuleImport(StringRef module, bool isExported,
931935
llvm::StringSet<> *alreadyAddedModules = nullptr);
932936

933937
/// Add all of the module imports in the given source
@@ -937,13 +941,13 @@ class ModuleDependencyInfo {
937941
const SourceManager *sourceManager);
938942

939943
/// Add a dependency on the given module, if it was not already in the set.
940-
void addModuleImport(ImportPath::Module module,
944+
void addModuleImport(ImportPath::Module module, bool isExported,
941945
llvm::StringSet<> *alreadyAddedModules = nullptr,
942946
const SourceManager *sourceManager = nullptr,
943947
SourceLoc sourceLocation = SourceLoc());
944948

945949
/// Add a dependency on the given module, if it was not already in the set.
946-
void addModuleImport(StringRef module,
950+
void addModuleImport(StringRef module, bool isExported,
947951
llvm::StringSet<> *alreadyAddedModules = nullptr,
948952
const SourceManager *sourceManager = nullptr,
949953
SourceLoc sourceLocation = SourceLoc());

‎include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,12 @@ using IsFrameworkField = BCFixed<1>;
5656
using IsSystemField = BCFixed<1>;
5757
/// A bit that indicates whether or not a module is that of a static archive
5858
using IsStaticField = BCFixed<1>;
59-
/// A bit taht indicates whether or not a link library is a force-load one
59+
/// A bit that indicates whether or not a link library is a force-load one
6060
using IsForceLoadField = BCFixed<1>;
61-
/// A bit taht indicates whether or not an import statement is optional
61+
/// A bit that indicates whether or not an import statement is optional
6262
using IsOptionalImport = BCFixed<1>;
63+
/// A bit that indicates whether or not an import statement is @_exported
64+
using IsExportedImport = BCFixed<1>;
6365

6466
/// Source location fields
6567
using LineNumberField = BCFixed<32>;
@@ -176,7 +178,8 @@ using ImportStatementLayout =
176178
IdentifierIDField, // bufferIdentifier
177179
LineNumberField, // lineNumber
178180
ColumnNumberField, // columnNumber
179-
IsOptionalImport // isOptional
181+
IsOptionalImport, // isOptional
182+
IsExportedImport // isExported
180183
>;
181184
using ImportStatementArrayLayout =
182185
BCRecordLayout<IMPORT_STATEMENT_ARRAY_NODE, IdentifierIDArryField>;

‎include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
171171

172172
struct BinaryModuleImports {
173173
llvm::StringSet<> moduleImports;
174+
llvm::StringSet<> exportedModules;
174175
std::string headerImport;
175176
};
176177

@@ -185,7 +186,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
185186

186187
/// If the module has a package name matching the one
187188
/// specified, return a set of package-only imports for this module.
188-
static llvm::ErrorOr<llvm::StringSet<>>
189+
static llvm::ErrorOr<std::vector<ScannerImportStatementInfo>>
189190
getMatchingPackageOnlyImportsOfModule(Twine modulePath,
190191
bool isFramework,
191192
bool isRequiredOSSAModules,

‎lib/AST/ModuleDependencies.cpp

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,13 @@ bool ModuleDependencyInfo::isTestableImport(StringRef moduleName) const {
117117
}
118118

119119
void ModuleDependencyInfo::addOptionalModuleImport(
120-
StringRef module, llvm::StringSet<> *alreadyAddedModules) {
120+
StringRef module, bool isExported, llvm::StringSet<> *alreadyAddedModules) {
121121
if (!alreadyAddedModules || alreadyAddedModules->insert(module).second)
122-
storage->optionalModuleImports.push_back(module.str());
122+
storage->optionalModuleImports.push_back({module.str(), isExported});
123123
}
124124

125125
void ModuleDependencyInfo::addModuleImport(
126-
StringRef module, llvm::StringSet<> *alreadyAddedModules,
126+
StringRef module, bool isExported, llvm::StringSet<> *alreadyAddedModules,
127127
const SourceManager *sourceManager, SourceLoc sourceLocation) {
128128
auto scannerImportLocToDiagnosticLocInfo =
129129
[&sourceManager](SourceLoc sourceLocation) {
@@ -137,14 +137,16 @@ void ModuleDependencyInfo::addModuleImport(
137137
sourceManager->isOwning(sourceLocation);
138138

139139
if (alreadyAddedModules && alreadyAddedModules->contains(module)) {
140-
if (validSourceLocation) {
141-
// Find a prior import of this module and add import location
142-
for (auto &existingImport : storage->moduleImports) {
143-
if (existingImport.importIdentifier == module) {
140+
// Find a prior import of this module and add import location
141+
// and adjust whether or not this module is ever imported as exported
142+
for (auto &existingImport : storage->moduleImports) {
143+
if (existingImport.importIdentifier == module) {
144+
if (validSourceLocation) {
144145
existingImport.addImportLocation(
145-
scannerImportLocToDiagnosticLocInfo(sourceLocation));
146-
break;
146+
scannerImportLocToDiagnosticLocInfo(sourceLocation));
147147
}
148+
existingImport.isExported |= isExported;
149+
break;
148150
}
149151
}
150152
} else {
@@ -153,15 +155,15 @@ void ModuleDependencyInfo::addModuleImport(
153155

154156
if (validSourceLocation)
155157
storage->moduleImports.push_back(ScannerImportStatementInfo(
156-
module.str(), scannerImportLocToDiagnosticLocInfo(sourceLocation)));
158+
module.str(), isExported, scannerImportLocToDiagnosticLocInfo(sourceLocation)));
157159
else
158160
storage->moduleImports.push_back(
159-
ScannerImportStatementInfo(module.str()));
161+
ScannerImportStatementInfo(module.str(), isExported));
160162
}
161163
}
162164

163165
void ModuleDependencyInfo::addModuleImport(
164-
ImportPath::Module module, llvm::StringSet<> *alreadyAddedModules,
166+
ImportPath::Module module, bool isExported, llvm::StringSet<> *alreadyAddedModules,
165167
const SourceManager *sourceManager, SourceLoc sourceLocation) {
166168
std::string ImportedModuleName = module.front().Item.str().str();
167169
auto submodulePath = module.getSubmodulePath();
@@ -174,7 +176,7 @@ void ModuleDependencyInfo::addModuleImport(
174176
alreadyAddedModules);
175177
}
176178

177-
addModuleImport(ImportedModuleName, alreadyAddedModules,
179+
addModuleImport(ImportedModuleName, isExported, alreadyAddedModules,
178180
sourceManager, sourceLocation);
179181
}
180182

@@ -203,7 +205,8 @@ void ModuleDependencyInfo::addModuleImports(
203205
importDecl->isExported()))
204206
continue;
205207

206-
addModuleImport(realPath, &alreadyAddedModules, sourceManager,
208+
addModuleImport(realPath, importDecl->isExported(),
209+
&alreadyAddedModules, sourceManager,
207210
importDecl->getLoc());
208211

209212
// Additionally, keep track of which dependencies of a Source

‎lib/ClangImporter/ClangModuleDependencyScanner.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,10 @@ ModuleDependencyVector ClangImporter::bridgeClangModuleDependencies(
281281

282282
std::vector<ModuleDependencyID> directDependencyIDs;
283283
for (const auto &moduleName : clangModuleDep.ClangModuleDeps) {
284-
dependencies.addModuleImport(moduleName.ModuleName, &alreadyAddedModules);
284+
// FIXME: This assumes, conservatively, that all Clang module imports
285+
// are exported. We need to fix this once the clang scanner gains the appropriate
286+
// API to query this.
287+
dependencies.addModuleImport(moduleName.ModuleName, /* isExported */ true, &alreadyAddedModules);
285288
// It is safe to assume that all dependencies of a Clang module are Clang modules.
286289
directDependencyIDs.push_back({moduleName.ModuleName, ModuleDependencyKind::Clang});
287290
}

‎lib/DependencyScan/ModuleDependencyCacheSerialization.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -240,10 +240,12 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
240240
&macroDependencies](ModuleDependencyInfo &moduleDep) {
241241
// Add imports of this module
242242
for (const auto &moduleName : currentModuleImports)
243-
moduleDep.addModuleImport(moduleName.importIdentifier);
243+
moduleDep.addModuleImport(moduleName.importIdentifier,
244+
moduleName.isExported);
244245
// Add optional imports of this module
245246
for (const auto &moduleName : currentOptionalModuleImports)
246-
moduleDep.addOptionalModuleImport(moduleName.importIdentifier);
247+
moduleDep.addOptionalModuleImport(moduleName.importIdentifier,
248+
moduleName.isExported);
247249

248250
// Add qualified dependencies of this module
249251
moduleDep.setImportedClangDependencies(importedClangDependenciesIDs);
@@ -408,10 +410,10 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
408410
case IMPORT_STATEMENT_NODE: {
409411
unsigned importIdentifierID, bufferIdentifierID;
410412
unsigned lineNumber, columnNumber;
411-
bool isOptional;
413+
bool isOptional, isExported;
412414
ImportStatementLayout::readRecord(Scratch, importIdentifierID,
413415
bufferIdentifierID, lineNumber,
414-
columnNumber, isOptional);
416+
columnNumber, isOptional, isExported);
415417
auto importIdentifier = getIdentifier(importIdentifierID);
416418
if (!importIdentifier)
417419
llvm::report_fatal_error("Bad import statement info: no import name");
@@ -421,7 +423,7 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
421423
llvm::report_fatal_error(
422424
"Bad import statement info: no buffer identifier");
423425
ImportStatements.push_back(ScannerImportStatementInfo(
424-
importIdentifier.value(),
426+
importIdentifier.value(), isExported,
425427
ScannerImportStatementInfo::ImportDiagnosticLocationInfo(
426428
bufferIdentifier.value(), lineNumber, columnNumber)));
427429
break;
@@ -1443,7 +1445,7 @@ unsigned ModuleDependenciesCacheSerializer::writeImportStatementInfos(
14431445
Out, ScratchRecord, AbbrCodes[ImportStatementLayout::Code],
14441446
getIdentifier(importInfo.importIdentifier),
14451447
getIdentifier(importLoc.bufferIdentifier), importLoc.lineNumber,
1446-
importLoc.columnNumber, isOptional);
1448+
importLoc.columnNumber, isOptional, importInfo.isExported);
14471449
count++;
14481450
}
14491451
};

‎lib/DependencyScan/ModuleDependencyScanner.cpp

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,7 @@ ModuleDependencyScanner::getMainModuleDependencyInfo(ModuleDecl *mainModule) {
412412
// Add any implicit module names.
413413
for (const auto &import : importInfo.AdditionalUnloadedImports) {
414414
mainDependencies.addModuleImport(import.module.getModulePath(),
415+
import.options.contains(ImportFlags::Exported),
415416
&alreadyAddedModules,
416417
&ScanASTContext.SourceMgr);
417418
}
@@ -420,6 +421,7 @@ ModuleDependencyScanner::getMainModuleDependencyInfo(ModuleDecl *mainModule) {
420421
for (const auto &import : importInfo.AdditionalImports) {
421422
mainDependencies.addModuleImport(
422423
import.module.importedModule->getNameStr(),
424+
import.options.contains(ImportFlags::Exported),
423425
&alreadyAddedModules);
424426
}
425427

@@ -432,6 +434,7 @@ ModuleDependencyScanner::getMainModuleDependencyInfo(ModuleDecl *mainModule) {
432434
// add a dependency with the same name to trigger the search.
433435
if (importInfo.ShouldImportUnderlyingModule) {
434436
mainDependencies.addModuleImport(mainModule->getName().str(),
437+
/* isExported */ true,
435438
&alreadyAddedModules);
436439
}
437440

@@ -441,6 +444,7 @@ ModuleDependencyScanner::getMainModuleDependencyInfo(ModuleDecl *mainModule) {
441444
for (const auto &tbdSymbolModule :
442445
ScanCompilerInvocation.getTBDGenOptions().embedSymbolsFromModules) {
443446
mainDependencies.addModuleImport(tbdSymbolModule,
447+
/* isExported */ false,
444448
&alreadyAddedModules);
445449
}
446450
}
@@ -586,18 +590,19 @@ static void discoverCrossImportOverlayFiles(
586590
mainModuleName.str(), ModuleDependencyKind::SwiftSource});
587591

588592
llvm::StringMap<ModuleDependencyIDSet> perSourceFileDependencies;
589-
const ModuleDependencyIDSet directSwiftDepsSet{
593+
const ModuleDependencyIDSet mainModuleDirectSwiftDepsSet{
590594
mainModuleInfo.getImportedSwiftDependencies().begin(),
591595
mainModuleInfo.getImportedSwiftDependencies().end()};
592-
const ModuleDependencyIDSet directClangDepsSet{
596+
const ModuleDependencyIDSet mainModuleDirectClangDepsSet{
593597
mainModuleInfo.getImportedClangDependencies().begin(),
594598
mainModuleInfo.getImportedClangDependencies().end()};
595599

596600
// A utility to map an import identifier to one of the
597601
// known resolved module dependencies
598602
auto getModuleIDForImportIdentifier =
599-
[directSwiftDepsSet, directClangDepsSet](
600-
const std::string &importIdentifierStr) -> ModuleDependencyID {
603+
[](const std::string &importIdentifierStr,
604+
const ModuleDependencyIDSet &directSwiftDepsSet,
605+
const ModuleDependencyIDSet &directClangDepsSet) -> ModuleDependencyID {
601606
if (auto textualDepIt = directSwiftDepsSet.find(
602607
{importIdentifierStr, ModuleDependencyKind::SwiftInterface});
603608
textualDepIt != directSwiftDepsSet.end())
@@ -617,8 +622,9 @@ static void discoverCrossImportOverlayFiles(
617622
// Collect the set of directly-imported module dependencies
618623
// for each source file in the source module under scan.
619624
for (const auto &import : mainModuleInfo.getModuleImports()) {
620-
auto importResolvedModuleID =
621-
getModuleIDForImportIdentifier(import.importIdentifier);
625+
auto importResolvedModuleID = getModuleIDForImportIdentifier(
626+
import.importIdentifier, mainModuleDirectSwiftDepsSet,
627+
mainModuleDirectClangDepsSet);
622628
for (const auto &importLocation : import.importLocations)
623629
perSourceFileDependencies[importLocation.bufferIdentifier].insert(
624630
importResolvedModuleID);
@@ -636,19 +642,29 @@ static void discoverCrossImportOverlayFiles(
636642
auto moduleID = worklist.pop_back_val();
637643
perSourceFileDependencies[bufferIdentifier].insert(moduleID);
638644
if (isSwiftDependencyKind(moduleID.Kind)) {
639-
for (const auto &directSwiftDepID :
640-
cache.getImportedSwiftDependencies(moduleID)) {
641-
if (perSourceFileDependencies[bufferIdentifier].count(directSwiftDepID))
642-
continue;
643-
worklist.push_back(directSwiftDepID);
645+
auto moduleInfo = cache.findKnownDependency(moduleID);
646+
if (llvm::any_of(moduleInfo.getModuleImports(),
647+
[](const ScannerImportStatementInfo &importInfo) {
648+
return importInfo.isExported;
649+
})) {
650+
const ModuleDependencyIDSet directSwiftDepsSet{
651+
moduleInfo.getImportedSwiftDependencies().begin(),
652+
moduleInfo.getImportedSwiftDependencies().end()};
653+
const ModuleDependencyIDSet directClangDepsSet{
654+
moduleInfo.getImportedClangDependencies().begin(),
655+
moduleInfo.getImportedClangDependencies().end()};
656+
for (const auto &import : moduleInfo.getModuleImports()) {
657+
if (import.isExported) {
658+
auto importResolvedDepID = getModuleIDForImportIdentifier(
659+
import.importIdentifier, directSwiftDepsSet,
660+
directClangDepsSet);
661+
if (!perSourceFileDependencies[bufferIdentifier].count(
662+
importResolvedDepID))
663+
worklist.push_back(importResolvedDepID);
664+
}
665+
}
644666
}
645667
}
646-
for (const auto &directClangDepID :
647-
cache.getImportedClangDependencies(moduleID)) {
648-
if (perSourceFileDependencies[bufferIdentifier].count(directClangDepID))
649-
continue;
650-
worklist.push_back(directClangDepID);
651-
}
652668
}
653669
}
654670

@@ -1354,7 +1370,8 @@ void ModuleDependencyScanner::resolveCrossImportOverlayDependencies(
13541370
ModuleDependencyInfo::forSwiftSourceModule();
13551371
std::for_each(newOverlays.begin(), newOverlays.end(),
13561372
[&](Identifier modName) {
1357-
dummyMainDependencies.addModuleImport(modName.str());
1373+
dummyMainDependencies.addModuleImport(modName.str(),
1374+
/* isExported */ false);
13581375
});
13591376

13601377
// Record the dummy main module's direct dependencies. The dummy main module

‎lib/Serialization/ScanningLoaders.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ SwiftModuleScanner::scanInterfaceFile(Twine moduleInterfacePath,
276276
auto &imInfo = mainMod->getImplicitImportInfo();
277277
for (auto import : imInfo.AdditionalUnloadedImports) {
278278
Result->addModuleImport(import.module.getModulePath(),
279+
import.options.contains(ImportFlags::Exported),
279280
&alreadyAddedModules, &Ctx.SourceMgr);
280281
}
281282

@@ -301,8 +302,9 @@ SwiftModuleScanner::scanInterfaceFile(Twine moduleInterfacePath,
301302
return adjacentBinaryModulePackageOnlyImports.getError();
302303

303304
for (const auto &requiredImport : *adjacentBinaryModulePackageOnlyImports)
304-
if (!alreadyAddedModules.contains(requiredImport.getKey()))
305-
Result->addModuleImport(requiredImport.getKey(),
305+
if (!alreadyAddedModules.contains(requiredImport.importIdentifier))
306+
Result->addModuleImport(requiredImport.importIdentifier,
307+
requiredImport.isExported,
306308
&alreadyAddedModules);
307309
}
308310
}

0 commit comments

Comments
 (0)