From c1ba410e72b541439f608c3eb8a05d0423a537a8 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Mon, 10 Nov 2025 13:05:57 -0800 Subject: [PATCH 1/5] Using the compiler instance with context for by-name lookups. --- .../DependencyScan/ModuleDependencyScanner.h | 6 +++ .../ModuleDependencyScanner.cpp | 48 +++++++++++++++---- lib/DependencyScan/ScanDependencies.cpp | 9 ++++ 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/include/swift/DependencyScan/ModuleDependencyScanner.h b/include/swift/DependencyScan/ModuleDependencyScanner.h index 01cf52734b8ca..3a835135030e3 100644 --- a/include/swift/DependencyScan/ModuleDependencyScanner.h +++ b/include/swift/DependencyScan/ModuleDependencyScanner.h @@ -51,6 +51,9 @@ class ModuleDependencyScanningWorker { llvm::PrefixMapper *mapper, DiagnosticEngine &diags); private: + llvm::Error initializeClangScanningTool(); + llvm::Error finalizeClangScanningTool(); + /// Query dependency information for a named Clang module /// /// \param moduleName moduel identifier for the query @@ -229,6 +232,9 @@ class ModuleDependencyScanner { llvm::ErrorOr getMainModuleDependencyInfo(ModuleDecl *mainModule); + llvm::Error initializeWorkerClangScanningTool(); + llvm::Error finalizeWorkerClangScanningTool(); + /// Resolve module dependencies of the given module, computing a full /// transitive closure dependency graph. std::vector diff --git a/lib/DependencyScan/ModuleDependencyScanner.cpp b/lib/DependencyScan/ModuleDependencyScanner.cpp index f40137d3e6f75..894340a946c2b 100644 --- a/lib/DependencyScan/ModuleDependencyScanner.cpp +++ b/lib/DependencyScan/ModuleDependencyScanner.cpp @@ -296,6 +296,15 @@ ModuleDependencyScanningWorker::ModuleDependencyScanningWorker( workerCompilerInvocation->getSearchPathOptions().ExplicitSwiftModuleInputs); } +llvm::Error ModuleDependencyScanningWorker::initializeClangScanningTool() { + return clangScanningTool.initializeCompilerInstanceWithContext( + clangScanningWorkingDirectoryPath, clangScanningModuleCommandLineArgs); +} + +llvm::Error ModuleDependencyScanningWorker::finalizeClangScanningTool() { + return clangScanningTool.finalizeCompilerInstanceWithContext(); +} + SwiftModuleScannerQueryResult ModuleDependencyScanningWorker::scanFilesystemForSwiftModuleDependency( Identifier moduleName, bool isTestableImport) { @@ -309,18 +318,23 @@ ModuleDependencyScanningWorker::scanFilesystemForClangModuleDependency( LookupModuleOutputCallback lookupModuleOutput, const llvm::DenseSet &alreadySeenModules) { - auto clangModuleDependencies = clangScanningTool.getModuleDependencies( - moduleName.str(), clangScanningModuleCommandLineArgs, - clangScanningWorkingDirectoryPath, alreadySeenModules, - lookupModuleOutput); + // auto clangModuleDependencies = clangScanningTool.getModuleDependencies( + // moduleName.str(), clangScanningModuleCommandLineArgs, + // clangScanningWorkingDirectoryPath, alreadySeenModules, + // lookupModuleOutput); + auto clangModuleDependencies = + clangScanningTool.computeDependenciesByNameWithContext( + moduleName.str(), alreadySeenModules, lookupModuleOutput); if (!clangModuleDependencies) { - llvm::handleAllErrors(clangModuleDependencies.takeError(), [this, &moduleName]( - const llvm::StringError &E) { + llvm::handleAllErrors( + clangModuleDependencies.takeError(), + [this, &moduleName](const llvm::StringError &E) { auto &message = E.getMessage(); if (message.find("fatal error: module '" + moduleName.str().str() + - "' not found") == std::string::npos) - workerDiagnosticEngine->diagnose(SourceLoc(), diag::clang_dependency_scan_error, message); - }); + "' not found") == std::string::npos) + workerDiagnosticEngine->diagnose( + SourceLoc(), diag::clang_dependency_scan_error, message); + }); return std::nullopt; } @@ -560,6 +574,22 @@ ModuleDependencyScanner::ModuleDependencyScanner( DependencyTracker, CAS, ActionCache, PrefixMapper.get(), Diagnostics)); } +llvm::Error ModuleDependencyScanner::initializeWorkerClangScanningTool() { + for (auto &W : Workers) { + if (auto error = W->initializeClangScanningTool()) + return error; + } + return llvm::Error::success(); +} + +llvm::Error ModuleDependencyScanner::finalizeWorkerClangScanningTool() { + for (auto &W : Workers) { + if (auto error = W->finalizeClangScanningTool()) + return error; + } + return llvm::Error::success(); +} + static std::set collectBinarySwiftDeps(const ModuleDependenciesCache &cache) { std::set binarySwiftModuleDepIDs; diff --git a/lib/DependencyScan/ScanDependencies.cpp b/lib/DependencyScan/ScanDependencies.cpp index 2af0d2555772d..66a42f7e9ed98 100644 --- a/lib/DependencyScan/ScanDependencies.cpp +++ b/lib/DependencyScan/ScanDependencies.cpp @@ -1412,6 +1412,11 @@ performModuleScanImpl( instance->getDiags(), instance->getInvocation().getFrontendOptions().ParallelDependencyScan); + auto initError = scanner.initializeWorkerClangScanningTool(); + // TODO: fix error check! + if (initError) + llvm::consumeError(std::move(initError)); + // Identify imports of the main module and add an entry for it // to the dependency graph. auto mainModuleName = instance->getMainModule()->getNameStr(); @@ -1426,6 +1431,10 @@ performModuleScanImpl( if (diagnoseCycle(*instance, cache, mainModuleID)) return std::make_error_code(std::errc::not_supported); + auto finError = scanner.finalizeWorkerClangScanningTool(); + if (finError) + llvm::consumeError(std::move(finError)); + auto topologicallySortedModuleList = computeTopologicalSortOfExplicitDependencies(allModules, cache); From 59c3895db60938a0bfa93075ac9b160c938fa6d5 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Mon, 17 Nov 2025 15:47:14 -0800 Subject: [PATCH 2/5] Add error handling for compiler instance with context initialization and finalization. --- .../ModuleDependencyScanner.cpp | 7 +----- lib/DependencyScan/ScanDependencies.cpp | 22 ++++++++++++++----- unittests/DependencyScan/ModuleDeps.cpp | 2 +- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/lib/DependencyScan/ModuleDependencyScanner.cpp b/lib/DependencyScan/ModuleDependencyScanner.cpp index 894340a946c2b..44686115b829f 100644 --- a/lib/DependencyScan/ModuleDependencyScanner.cpp +++ b/lib/DependencyScan/ModuleDependencyScanner.cpp @@ -314,14 +314,9 @@ ModuleDependencyScanningWorker::scanFilesystemForSwiftModuleDependency( std::optional ModuleDependencyScanningWorker::scanFilesystemForClangModuleDependency( - Identifier moduleName, - LookupModuleOutputCallback lookupModuleOutput, + Identifier moduleName, LookupModuleOutputCallback lookupModuleOutput, const llvm::DenseSet &alreadySeenModules) { - // auto clangModuleDependencies = clangScanningTool.getModuleDependencies( - // moduleName.str(), clangScanningModuleCommandLineArgs, - // clangScanningWorkingDirectoryPath, alreadySeenModules, - // lookupModuleOutput); auto clangModuleDependencies = clangScanningTool.computeDependenciesByNameWithContext( moduleName.str(), alreadySeenModules, lookupModuleOutput); diff --git a/lib/DependencyScan/ScanDependencies.cpp b/lib/DependencyScan/ScanDependencies.cpp index 66a42f7e9ed98..a6e9ccbcbc40c 100644 --- a/lib/DependencyScan/ScanDependencies.cpp +++ b/lib/DependencyScan/ScanDependencies.cpp @@ -1413,9 +1413,14 @@ performModuleScanImpl( instance->getInvocation().getFrontendOptions().ParallelDependencyScan); auto initError = scanner.initializeWorkerClangScanningTool(); - // TODO: fix error check! - if (initError) - llvm::consumeError(std::move(initError)); + if (initError) { + llvm::handleAllErrors( + std::move(initError), [&](const llvm::StringError &E) { + instance->getDiags().diagnose( + SourceLoc(), diag::clang_dependency_scan_error, E.getMessage()); + }); + return std::make_error_code(std::errc::invalid_argument); + } // Identify imports of the main module and add an entry for it // to the dependency graph. @@ -1432,8 +1437,15 @@ performModuleScanImpl( return std::make_error_code(std::errc::not_supported); auto finError = scanner.finalizeWorkerClangScanningTool(); - if (finError) - llvm::consumeError(std::move(finError)); + if (finError) { + llvm::handleAllErrors(std::move(finError), [&](const llvm::StringError &E) { + instance->getDiags().diagnose( + SourceLoc(), diag::clang_dependency_scan_error, E.getMessage()); + }); + // TODO:it is not expected that the finialization fails. Maybe we should + // turn this into an assert. + return std::make_error_code(std::errc::not_supported); + } auto topologicallySortedModuleList = computeTopologicalSortOfExplicitDependencies(allModules, cache); diff --git a/unittests/DependencyScan/ModuleDeps.cpp b/unittests/DependencyScan/ModuleDeps.cpp index e932fd8bf937a..77d585f03eb95 100644 --- a/unittests/DependencyScan/ModuleDeps.cpp +++ b/unittests/DependencyScan/ModuleDeps.cpp @@ -514,6 +514,6 @@ TEST_F(ScanTest, TestStressConcurrentDiagnostics) { ASSERT_FALSE(DependenciesOrErr.getError()); auto Dependencies = DependenciesOrErr.get(); auto Diagnostics = Dependencies->diagnostics; - ASSERT_TRUE(Diagnostics->count > 100); + ASSERT_TRUE(Diagnostics->count == 2); swiftscan_dependency_graph_dispose(Dependencies); } From a9ec4c71268bb26b92242f6097fe43fe8ae6fe14 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Wed, 19 Nov 2025 09:24:54 -0800 Subject: [PATCH 3/5] Fix the unit test. --- unittests/DependencyScan/ModuleDeps.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/DependencyScan/ModuleDeps.cpp b/unittests/DependencyScan/ModuleDeps.cpp index 77d585f03eb95..0ed70ea79ac66 100644 --- a/unittests/DependencyScan/ModuleDeps.cpp +++ b/unittests/DependencyScan/ModuleDeps.cpp @@ -514,6 +514,6 @@ TEST_F(ScanTest, TestStressConcurrentDiagnostics) { ASSERT_FALSE(DependenciesOrErr.getError()); auto Dependencies = DependenciesOrErr.get(); auto Diagnostics = Dependencies->diagnostics; - ASSERT_TRUE(Diagnostics->count == 2); + ASSERT_TRUE(Diagnostics->count >= 1); swiftscan_dependency_graph_dispose(Dependencies); } From c91014c870cc5b8054118443fa239a3c6558060c Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Mon, 1 Dec 2025 20:08:50 -0800 Subject: [PATCH 4/5] Adding some documentation on the clang scanning tool's initialization method. --- include/swift/DependencyScan/ModuleDependencyScanner.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/swift/DependencyScan/ModuleDependencyScanner.h b/include/swift/DependencyScan/ModuleDependencyScanner.h index 3a835135030e3..595713ddcfaec 100644 --- a/include/swift/DependencyScan/ModuleDependencyScanner.h +++ b/include/swift/DependencyScan/ModuleDependencyScanner.h @@ -51,6 +51,16 @@ class ModuleDependencyScanningWorker { llvm::PrefixMapper *mapper, DiagnosticEngine &diags); private: + /// Initialize/finalize the clang compiler scanning tool. + /// Behind the scenes, the clang scanning tool maintains + /// a single clang compiler instance to perform all by-name + /// dependency scans. initializeClangScanningTool() initializes + /// the clang compiler instance, and returns an error if the + /// initialization fails. Once successfully initialized, + /// the same clang compiler instance is reused whenever + /// scanFilesystemForClangModuleDependency is called, + /// throughout the lifetime of the ModuleDependencyScanningWorker + /// instance. llvm::Error initializeClangScanningTool(); llvm::Error finalizeClangScanningTool(); From dac0a53156526576f02cc884acf8e846d83bad91 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Mon, 15 Dec 2025 18:59:39 -0800 Subject: [PATCH 5/5] Refactoring the creation of the ModuleDependencyScanner. --- .../DependencyScan/ModuleDependencyScanner.h | 30 +++++++----- .../ModuleDependencyScanner.cpp | 33 +++++++++++++ lib/DependencyScan/ScanDependencies.cpp | 48 ++++++------------- 3 files changed, 66 insertions(+), 45 deletions(-) diff --git a/include/swift/DependencyScan/ModuleDependencyScanner.h b/include/swift/DependencyScan/ModuleDependencyScanner.h index 595713ddcfaec..a741cc3e4c478 100644 --- a/include/swift/DependencyScan/ModuleDependencyScanner.h +++ b/include/swift/DependencyScan/ModuleDependencyScanner.h @@ -228,23 +228,16 @@ class ModuleDependencyIssueReporter { class ModuleDependencyScanner { public: - ModuleDependencyScanner(SwiftDependencyScanningService &ScanningService, - ModuleDependenciesCache &Cache, - const CompilerInvocation &ScanCompilerInvocation, - const SILOptions &SILOptions, - ASTContext &ScanASTContext, - DependencyTracker &DependencyTracker, - std::shared_ptr CAS, - std::shared_ptr ActionCache, - DiagnosticEngine &Diagnostics, bool ParallelScan); + static llvm::ErrorOr> + create(SwiftDependencyScanningService &service, CompilerInstance *instance, + ModuleDependenciesCache &cache); + + ~ModuleDependencyScanner(); /// Identify the scanner invocation's main module's dependencies llvm::ErrorOr getMainModuleDependencyInfo(ModuleDecl *mainModule); - llvm::Error initializeWorkerClangScanningTool(); - llvm::Error finalizeWorkerClangScanningTool(); - /// Resolve module dependencies of the given module, computing a full /// transitive closure dependency graph. std::vector @@ -281,6 +274,19 @@ class ModuleDependencyScanner { } private: + // Private methods that create, initialize and finalize the scanner. + ModuleDependencyScanner(SwiftDependencyScanningService &ScanningService, + ModuleDependenciesCache &Cache, + const CompilerInvocation &ScanCompilerInvocation, + const SILOptions &SILOptions, + ASTContext &ScanASTContext, + DependencyTracker &DependencyTracker, + std::shared_ptr CAS, + std::shared_ptr ActionCache, + DiagnosticEngine &Diagnostics, bool ParallelScan); + llvm::Error initializeWorkerClangScanningTool(); + llvm::Error finalizeWorkerClangScanningTool(); + /// Main routine that computes imported module dependency transitive /// closure for the given module. /// 1. Swift modules imported directly or via another Swift dependency diff --git a/lib/DependencyScan/ModuleDependencyScanner.cpp b/lib/DependencyScan/ModuleDependencyScanner.cpp index 41665edf6fc41..8c5c83e2bad9b 100644 --- a/lib/DependencyScan/ModuleDependencyScanner.cpp +++ b/lib/DependencyScan/ModuleDependencyScanner.cpp @@ -525,6 +525,34 @@ SwiftDependencyTracker::createTreeFromDependencies() { return *includeTreeList; } +llvm::ErrorOr> +ModuleDependencyScanner::create(SwiftDependencyScanningService &service, + CompilerInstance *instance, + ModuleDependenciesCache &cache) { + auto scanner = + std::unique_ptr(new ModuleDependencyScanner( + service, cache, instance->getInvocation(), instance->getSILOptions(), + instance->getASTContext(), *instance->getDependencyTracker(), + instance->getSharedCASInstance(), instance->getSharedCacheInstance(), + instance->getDiags(), + instance->getInvocation() + .getFrontendOptions() + .ParallelDependencyScan)); + + auto initError = scanner->initializeWorkerClangScanningTool(); + + if (initError) { + llvm::handleAllErrors( + std::move(initError), [&](const llvm::StringError &E) { + instance->getDiags().diagnose( + SourceLoc(), diag::clang_dependency_scan_error, E.getMessage()); + }); + return std::make_error_code(std::errc::invalid_argument); + } + + return scanner; +} + ModuleDependencyScanner::ModuleDependencyScanner( SwiftDependencyScanningService &ScanningService, ModuleDependenciesCache &Cache, @@ -569,6 +597,11 @@ ModuleDependencyScanner::ModuleDependencyScanner( DependencyTracker, CAS, ActionCache, PrefixMapper.get(), Diagnostics)); } +ModuleDependencyScanner::~ModuleDependencyScanner() { + auto finError = finalizeWorkerClangScanningTool(); + assert(!finError && "ClangScanningTool finalization must succeed."); +} + llvm::Error ModuleDependencyScanner::initializeWorkerClangScanningTool() { for (auto &W : Workers) { if (auto error = W->initializeClangScanningTool()) diff --git a/lib/DependencyScan/ScanDependencies.cpp b/lib/DependencyScan/ScanDependencies.cpp index a6e9ccbcbc40c..b6379a55ca892 100644 --- a/lib/DependencyScan/ScanDependencies.cpp +++ b/lib/DependencyScan/ScanDependencies.cpp @@ -1405,22 +1405,13 @@ performModuleScanImpl( } } - auto scanner = ModuleDependencyScanner( - service, cache, instance->getInvocation(), instance->getSILOptions(), - instance->getASTContext(), *instance->getDependencyTracker(), - instance->getSharedCASInstance(), instance->getSharedCacheInstance(), - instance->getDiags(), - instance->getInvocation().getFrontendOptions().ParallelDependencyScan); - - auto initError = scanner.initializeWorkerClangScanningTool(); - if (initError) { - llvm::handleAllErrors( - std::move(initError), [&](const llvm::StringError &E) { - instance->getDiags().diagnose( - SourceLoc(), diag::clang_dependency_scan_error, E.getMessage()); - }); - return std::make_error_code(std::errc::invalid_argument); - } + auto expectedScannerPtr = + ModuleDependencyScanner::create(service, instance, cache); + + if (!expectedScannerPtr) + return expectedScannerPtr.getError(); + + auto &scanner = **expectedScannerPtr; // Identify imports of the main module and add an entry for it // to the dependency graph. @@ -1436,17 +1427,6 @@ performModuleScanImpl( if (diagnoseCycle(*instance, cache, mainModuleID)) return std::make_error_code(std::errc::not_supported); - auto finError = scanner.finalizeWorkerClangScanningTool(); - if (finError) { - llvm::handleAllErrors(std::move(finError), [&](const llvm::StringError &E) { - instance->getDiags().diagnose( - SourceLoc(), diag::clang_dependency_scan_error, E.getMessage()); - }); - // TODO:it is not expected that the finialization fails. Maybe we should - // turn this into an assert. - return std::make_error_code(std::errc::not_supported); - } - auto topologicallySortedModuleList = computeTopologicalSortOfExplicitDependencies(allModules, cache); @@ -1478,12 +1458,14 @@ static llvm::ErrorOr performModulePrescanImpl( ModuleDependenciesCache &cache, DepScanInMemoryDiagnosticCollector *diagnosticCollector) { // Setup the scanner - auto scanner = ModuleDependencyScanner( - service, cache, instance->getInvocation(), instance->getSILOptions(), - instance->getASTContext(), *instance->getDependencyTracker(), - instance->getSharedCASInstance(), instance->getSharedCacheInstance(), - instance->getDiags(), - instance->getInvocation().getFrontendOptions().ParallelDependencyScan); + auto expectedScannerPtr = + ModuleDependencyScanner::create(service, instance, cache); + + if (!expectedScannerPtr) + return expectedScannerPtr.getError(); + + auto &scanner = **expectedScannerPtr; + // Execute import prescan, and write JSON output to the output stream auto mainDependencies = scanner.getMainModuleDependencyInfo(instance->getMainModule());