Skip to content

Commit 49bca81

Browse files
artemcmartemcm
committed
[Dependency Scanning] Scan header inputs of binary Swift moduel dependencies
Otherwise they may have module dependencies of their own which will not be detected by the scanner and included in the list of explicit inputs for compilation.
1 parent 918bd3d commit 49bca81

12 files changed

+278
-201
lines changed

‎include/swift/AST/ModuleDependencies.h

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ class SwiftBinaryModuleDependencyStorage : public ModuleDependencyInfoStorageBas
332332
: ModuleDependencyInfoStorageBase(ModuleDependencyKind::SwiftBinary,
333333
moduleImports, optionalModuleImports, moduleCacheKey),
334334
compiledModulePath(compiledModulePath), moduleDocPath(moduleDocPath),
335-
sourceInfoPath(sourceInfoPath), preCompiledBridgingHeaderPaths(headerImports),
335+
sourceInfoPath(sourceInfoPath), headerImports(headerImports),
336336
isFramework(isFramework) {}
337337

338338
ModuleDependencyInfoStorageBase *clone() const override {
@@ -348,8 +348,14 @@ class SwiftBinaryModuleDependencyStorage : public ModuleDependencyInfoStorageBas
348348
/// The path to the .swiftSourceInfo file.
349349
const std::string sourceInfoPath;
350350

351-
/// The paths of all the .pch dependencies of this module.
352-
const std::vector<std::string> preCompiledBridgingHeaderPaths;
351+
/// The paths of all the .h dependencies of this module.
352+
const std::vector<std::string> headerImports;
353+
354+
/// Source files on which the header inputs depend.
355+
std::vector<std::string> headerSourceFiles;
356+
357+
/// (Clang) modules on which the header inputs depend.
358+
std::vector<std::string> headerModuleDependencies;
353359

354360
/// A flag that indicates this dependency is a framework
355361
const bool isFramework;
@@ -598,6 +604,26 @@ class ModuleDependencyInfo {
598604
return storage->swiftOverlayDependencies;
599605
}
600606

607+
const ArrayRef<std::string> getHeaderInputSourceFiles() const {
608+
if (auto *detail = getAsSwiftInterfaceModule())
609+
return detail->textualModuleDetails.bridgingSourceFiles;
610+
else if (auto *detail = getAsSwiftSourceModule())
611+
return detail->textualModuleDetails.bridgingSourceFiles;
612+
else if (auto *detail = getAsSwiftBinaryModule())
613+
return detail->headerSourceFiles;
614+
return {};
615+
}
616+
617+
const ArrayRef<std::string> getHeaderDependencies() const {
618+
if (auto *detail = getAsSwiftInterfaceModule())
619+
return detail->textualModuleDetails.bridgingModuleDependencies;
620+
else if (auto *detail = getAsSwiftSourceModule())
621+
return detail->textualModuleDetails.bridgingModuleDependencies;
622+
else if (auto *detail = getAsSwiftBinaryModule())
623+
return detail->headerModuleDependencies;
624+
return {};
625+
}
626+
601627
std::vector<std::string> getCommandline() const {
602628
if (auto *detail = getAsClangModule())
603629
return detail->buildCommandLine;
@@ -751,12 +777,12 @@ class ModuleDependencyInfo {
751777
/// Add source files
752778
void addSourceFile(StringRef sourceFile);
753779

754-
/// Add source files that the bridging header depends on.
755-
void addBridgingSourceFile(StringRef bridgingSourceFile);
780+
/// Add source files that the header input depends on.
781+
void addHeaderSourceFile(StringRef bridgingSourceFile);
756782

757-
/// Add (Clang) module on which the bridging header depends.
758-
void addBridgingModuleDependency(StringRef module,
759-
llvm::StringSet<> &alreadyAddedModules);
783+
/// Add (Clang) modules on which a non-bridging header input depends.
784+
void addHeaderInputModuleDependency(StringRef module,
785+
llvm::StringSet<> &alreadyAddedModules);
760786

761787
/// Add bridging header include tree.
762788
void addBridgingHeaderIncludeTree(StringRef ID);

‎include/swift/ClangImporter/ClangImporter.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -465,22 +465,24 @@ class ClangImporter final : public ClangModuleLoader {
465465
ModuleDependencyInfo &MDI,
466466
const clang::tooling::dependencies::TranslationUnitDeps &deps);
467467

468-
/// Add dependency information for the bridging header.
468+
/// Add dependency information for header dependencies
469+
/// of a binary Swift module.
469470
///
470471
/// \param moduleID the name of the Swift module whose dependency
471472
/// information will be augmented with information about the given
472-
/// bridging header.
473+
/// textual header inputs.
473474
///
474475
/// \param clangScanningTool The clang dependency scanner.
475476
///
476477
/// \param cache The module dependencies cache to update, with information
477478
/// about new Clang modules discovered along the way.
478479
///
479480
/// \returns \c true if an error occurred, \c false otherwise
480-
bool addBridgingHeaderDependencies(
481+
bool addHeaderDependencies(
481482
ModuleDependencyID moduleID,
482483
clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,
483484
ModuleDependenciesCache &cache);
485+
484486
clang::TargetInfo &getModuleAvailabilityTarget() const override;
485487
clang::ASTContext &getClangASTContext() const override;
486488
clang::Preprocessor &getClangPreprocessor() const override;

‎include/swift/DependencyScan/DependencyScanImpl.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,15 @@ typedef struct {
128128
/// Clang module dependencies
129129
swiftscan_string_set_t *swift_overlay_module_dependencies;
130130

131-
/// (Clang) header dependencies of this binary module.
132-
/// Typically pre-compiled bridging header.
131+
/// (Clang) header (.h) dependencies of this binary module.
133132
swiftscan_string_set_t *header_dependencies;
134133

134+
/// (Clang) module dependencies of the textual header inputs
135+
swiftscan_string_set_t *header_dependencies_module_dependnecies;
136+
137+
/// Source files included by the header dependencies of this binary module
138+
swiftscan_string_set_t *header_dependencies_source_files;
139+
135140
/// A flag to indicate whether or not this module is a framework.
136141
bool is_framework;
137142

‎include/swift/DependencyScan/ModuleDependencyScanner.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,9 @@ class ModuleDependencyScanner {
107107
ModuleDependenciesCache &cache,
108108
ModuleDependencyIDSetVector &directDependencies);
109109

110-
/// If a module has a bridging header, execute a dependency scan
110+
/// If a module has a bridging header or other header inputs, execute a dependency scan
111111
/// on it and record the dependencies.
112-
void resolveBridgingHeaderDependencies(
112+
void resolveHeaderDependencies(
113113
const ModuleDependencyID &moduleID, ModuleDependenciesCache &cache,
114114
std::vector<std::string> &allClangModules,
115115
llvm::StringSet<> &alreadyKnownModules,

‎include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ using SwiftInterfaceModuleDetailsLayout =
141141
FileIDField, // bridgingHeaderFile
142142
FileIDArrayIDField, // sourceFiles
143143
FileIDArrayIDField, // bridgingSourceFiles
144-
FileIDArrayIDField, // bridgingModuleDependencies
144+
IdentifierIDField, // bridgingModuleDependencies
145145
DependencyIDArrayIDField, // swiftOverlayDependencies
146146
IdentifierIDField, // CASFileSystemRootID
147147
IdentifierIDField, // bridgingHeaderIncludeTree
@@ -169,6 +169,8 @@ using SwiftBinaryModuleDetailsLayout =
169169
FileIDField, // moduleSourceInfoPath
170170
DependencyIDArrayIDField, // swiftOverlayDependencies
171171
ImportArrayIDField, // headerImports
172+
IdentifierIDField, // headerModuleDependencies
173+
FileIDArrayIDField, // headerSourceFiles
172174
IsFrameworkField, // isFramework
173175
IdentifierIDField // moduleCacheKey
174176
>;

‎lib/AST/ModuleDependencies.cpp

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ std::optional<std::string> ModuleDependencyInfo::getBridgingHeader() const {
197197
return swiftSourceStorage->textualModuleDetails.bridgingHeaderFile;
198198
}
199199
default:
200-
llvm_unreachable("Unexpected dependency kind");
200+
return std::nullopt;
201201
}
202202
}
203203

@@ -325,7 +325,7 @@ void ModuleDependencyInfo::addBridgingHeader(StringRef bridgingHeader) {
325325
}
326326

327327
/// Add source files that the bridging header depends on.
328-
void ModuleDependencyInfo::addBridgingSourceFile(StringRef bridgingSourceFile) {
328+
void ModuleDependencyInfo::addHeaderSourceFile(StringRef bridgingSourceFile) {
329329
switch (getKind()) {
330330
case swift::ModuleDependencyKind::SwiftInterface: {
331331
auto swiftInterfaceStorage =
@@ -337,7 +337,14 @@ void ModuleDependencyInfo::addBridgingSourceFile(StringRef bridgingSourceFile) {
337337
case swift::ModuleDependencyKind::SwiftSource: {
338338
auto swiftSourceStorage =
339339
cast<SwiftSourceModuleDependenciesStorage>(storage.get());
340-
swiftSourceStorage->textualModuleDetails.bridgingSourceFiles.push_back(bridgingSourceFile.str());
340+
swiftSourceStorage->textualModuleDetails.bridgingSourceFiles.push_back(
341+
bridgingSourceFile.str());
342+
break;
343+
}
344+
case swift::ModuleDependencyKind::SwiftBinary: {
345+
auto swiftBinaryStorage =
346+
cast<SwiftBinaryModuleDependencyStorage>(storage.get());
347+
swiftBinaryStorage->headerSourceFiles.push_back(bridgingSourceFile.str());
341348
break;
342349
}
343350
default:
@@ -380,21 +387,30 @@ void ModuleDependencyInfo::addSourceFile(StringRef sourceFile) {
380387
}
381388

382389
/// Add (Clang) module on which the bridging header depends.
383-
void ModuleDependencyInfo::addBridgingModuleDependency(
390+
void ModuleDependencyInfo::addHeaderInputModuleDependency(
384391
StringRef module, llvm::StringSet<> &alreadyAddedModules) {
385392
switch (getKind()) {
386393
case swift::ModuleDependencyKind::SwiftInterface: {
387394
auto swiftInterfaceStorage =
388395
cast<SwiftInterfaceModuleDependenciesStorage>(storage.get());
389396
if (alreadyAddedModules.insert(module).second)
390-
swiftInterfaceStorage->textualModuleDetails.bridgingModuleDependencies.push_back(module.str());
397+
swiftInterfaceStorage->textualModuleDetails.bridgingModuleDependencies
398+
.push_back(module.str());
391399
break;
392400
}
393401
case swift::ModuleDependencyKind::SwiftSource: {
394402
auto swiftSourceStorage =
395403
cast<SwiftSourceModuleDependenciesStorage>(storage.get());
396404
if (alreadyAddedModules.insert(module).second)
397-
swiftSourceStorage->textualModuleDetails.bridgingModuleDependencies.push_back(module.str());
405+
swiftSourceStorage->textualModuleDetails.bridgingModuleDependencies
406+
.push_back(module.str());
407+
break;
408+
}
409+
case swift::ModuleDependencyKind::SwiftBinary: {
410+
auto swiftBinaryStorage =
411+
cast<SwiftBinaryModuleDependencyStorage>(storage.get());
412+
if (alreadyAddedModules.insert(module).second)
413+
swiftBinaryStorage->headerModuleDependencies.push_back(module.str());
398414
break;
399415
}
400416
default:

‎lib/ClangImporter/ClangModuleDependencyScanner.cpp

Lines changed: 76 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -451,86 +451,96 @@ ClangImporter::getModuleDependencies(Identifier moduleName,
451451
});
452452
}
453453

454-
bool ClangImporter::addBridgingHeaderDependencies(
454+
bool ClangImporter::addHeaderDependencies(
455455
ModuleDependencyID moduleID,
456456
clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,
457457
ModuleDependenciesCache &cache) {
458458
auto &ctx = Impl.SwiftContext;
459459
auto optionalTargetModule = cache.findDependency(moduleID);
460460
assert(optionalTargetModule.has_value());
461461
auto targetModule = *(optionalTargetModule.value());
462-
463462
// If we've already recorded bridging header dependencies, we're done.
464-
if (auto swiftInterfaceDeps = targetModule.getAsSwiftInterfaceModule()) {
465-
if (!swiftInterfaceDeps->textualModuleDetails.bridgingSourceFiles.empty() ||
466-
!swiftInterfaceDeps->textualModuleDetails.bridgingModuleDependencies
467-
.empty())
468-
return false;
469-
} else if (auto swiftSourceDeps = targetModule.getAsSwiftSourceModule()) {
470-
if (!swiftSourceDeps->textualModuleDetails.bridgingSourceFiles.empty() ||
471-
!swiftSourceDeps->textualModuleDetails.bridgingModuleDependencies
472-
.empty())
473-
return false;
474-
} else {
475-
llvm_unreachable("Unexpected module dependency kind");
476-
}
463+
if (!targetModule.getHeaderDependencies().empty() ||
464+
!targetModule.getHeaderInputSourceFiles().empty())
465+
return false;
466+
467+
// Scan the specified textual header file and record its dependencies
468+
// into the cache
469+
auto scanHeaderDependencies =
470+
[&](StringRef headerPath) -> llvm::Expected<TranslationUnitDeps> {
471+
auto &ctx = Impl.SwiftContext;
472+
std::vector<std::string> commandLineArgs =
473+
getClangDepScanningInvocationArguments(ctx, StringRef(headerPath));
474+
auto optionalWorkingDir =
475+
computeClangWorkingDirectory(commandLineArgs, ctx);
476+
if (!optionalWorkingDir) {
477+
ctx.Diags.diagnose(SourceLoc(), diag::clang_dependency_scan_error,
478+
"Missing '-working-directory' argument");
479+
return llvm::errorCodeToError(
480+
std::error_code(errno, std::generic_category()));
481+
}
482+
std::string workingDir = *optionalWorkingDir;
483+
auto moduleCachePath = getModuleCachePathFromClang(getClangInstance());
484+
auto lookupModuleOutput =
485+
[moduleCachePath](const ModuleID &MID,
486+
ModuleOutputKind MOK) -> std::string {
487+
return moduleCacheRelativeLookupModuleOutput(MID, MOK, moduleCachePath);
488+
};
489+
auto dependencies = clangScanningTool.getTranslationUnitDependencies(
490+
commandLineArgs, workingDir, cache.getAlreadySeenClangModules(),
491+
lookupModuleOutput);
492+
if (!dependencies) {
493+
// FIXME: Route this to a normal diagnostic.
494+
llvm::logAllUnhandledErrors(dependencies.takeError(), llvm::errs());
495+
return dependencies.takeError();
496+
}
477497

478-
// Retrieve the bridging header.
479-
std::string bridgingHeader = *(targetModule.getBridgingHeader());
498+
// Record module dependencies for each new module we found.
499+
auto bridgedDeps = bridgeClangModuleDependencies(
500+
clangScanningTool, dependencies->ModuleGraph,
501+
cache.getModuleOutputPath(),
502+
[&cache](StringRef path) {
503+
return cache.getScanService().remapPath(path);
504+
});
505+
cache.recordDependencies(bridgedDeps);
480506

481-
// Determine the command-line arguments for dependency scanning.
482-
std::vector<std::string> commandLineArgs =
483-
getClangDepScanningInvocationArguments(ctx, StringRef(bridgingHeader));
484-
auto optionalWorkingDir = computeClangWorkingDirectory(commandLineArgs, ctx);
485-
if (!optionalWorkingDir) {
486-
ctx.Diags.diagnose(SourceLoc(), diag::clang_dependency_scan_error,
487-
"Missing '-working-directory' argument");
488-
return true;
489-
}
490-
std::string workingDir = *optionalWorkingDir;
507+
// Record dependencies for the source files the bridging header includes.
508+
for (const auto &fileDep : dependencies->FileDeps)
509+
targetModule.addHeaderSourceFile(fileDep);
491510

492-
auto moduleCachePath = getModuleCachePathFromClang(getClangInstance());
493-
auto lookupModuleOutput =
494-
[moduleCachePath](const ModuleID &MID,
495-
ModuleOutputKind MOK) -> std::string {
496-
return moduleCacheRelativeLookupModuleOutput(MID, MOK, moduleCachePath);
511+
// ... and all module dependencies.
512+
llvm::StringSet<> alreadyAddedModules;
513+
for (const auto &moduleDep : dependencies->ClangModuleDeps)
514+
targetModule.addHeaderInputModuleDependency(moduleDep.ModuleName,
515+
alreadyAddedModules);
516+
517+
return dependencies;
497518
};
498519

499-
auto clangModuleDependencies =
500-
clangScanningTool.getTranslationUnitDependencies(
501-
commandLineArgs, workingDir, cache.getAlreadySeenClangModules(),
502-
lookupModuleOutput);
503-
if (!clangModuleDependencies) {
504-
// FIXME: Route this to a normal diagnostic.
505-
llvm::logAllUnhandledErrors(clangModuleDependencies.takeError(), llvm::errs());
506-
return true;
520+
// - Textual module dependencies require us to process their bridging header.
521+
// - Binary module dependnecies may have arbitrary header inputs.
522+
if (targetModule.isTextualSwiftModule() &&
523+
!targetModule.getBridgingHeader()->empty()) {
524+
auto clangModuleDependencies =
525+
scanHeaderDependencies(*targetModule.getBridgingHeader());
526+
if (!clangModuleDependencies)
527+
return true;
528+
if (auto TreeID = clangModuleDependencies->IncludeTreeID)
529+
targetModule.addBridgingHeaderIncludeTree(*TreeID);
530+
recordBridgingHeaderOptions(targetModule, *clangModuleDependencies);
531+
// Update the cache with the new information for the module.
532+
cache.updateDependency(moduleID, targetModule);
533+
} else if (targetModule.isSwiftBinaryModule()) {
534+
auto swiftBinaryDeps = targetModule.getAsSwiftBinaryModule();
535+
if (!swiftBinaryDeps->headerImports.empty()) {
536+
auto clangModuleDependencies = scanHeaderDependencies(swiftBinaryDeps->headerImports.front());
537+
if (!clangModuleDependencies)
538+
return true;
539+
// TODO: CAS will require a header include tree for this.
540+
// Update the cache with the new information for the module.
541+
cache.updateDependency(moduleID, targetModule);
542+
}
507543
}
508544

509-
// Record module dependencies for each new module we found.
510-
auto bridgedDeps = bridgeClangModuleDependencies(
511-
clangScanningTool, clangModuleDependencies->ModuleGraph,
512-
cache.getModuleOutputPath(), [&cache](StringRef path) {
513-
return cache.getScanService().remapPath(path);
514-
});
515-
cache.recordDependencies(bridgedDeps);
516-
517-
// Record dependencies for the source files the bridging header includes.
518-
for (const auto &fileDep : clangModuleDependencies->FileDeps)
519-
targetModule.addBridgingSourceFile(fileDep);
520-
521-
// ... and all module dependencies.
522-
llvm::StringSet<> alreadyAddedModules;
523-
for (const auto &moduleDep : clangModuleDependencies->ClangModuleDeps)
524-
targetModule.addBridgingModuleDependency(moduleDep.ModuleName,
525-
alreadyAddedModules);
526-
527-
if (auto TreeID = clangModuleDependencies->IncludeTreeID)
528-
targetModule.addBridgingHeaderIncludeTree(*TreeID);
529-
530-
recordBridgingHeaderOptions(targetModule, *clangModuleDependencies);
531-
532-
// Update the cache with the new information for the module.
533-
cache.updateDependency(moduleID, targetModule);
534-
535545
return false;
536546
}

0 commit comments

Comments
 (0)