From c1ba410e72b541439f608c3eb8a05d0423a537a8 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Mon, 10 Nov 2025 13:05:57 -0800 Subject: [PATCH 1/2] 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/2] 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); }