From 554613b511baa2772e0f1871008fd124bdc48700 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Mon, 29 Sep 2025 16:46:02 -0700 Subject: [PATCH 1/5] Extract dependency scanning compiler instance initialization stepts into functions. --- .../DependencyScannerImpl.cpp | 257 +++++++++++++++--- .../DependencyScannerImpl.h | 59 +++- .../DependencyScanningWorker.cpp | 132 ++------- 3 files changed, 293 insertions(+), 155 deletions(-) diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp index d370bfd0dd10f..9a8fabaab42ec 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp @@ -9,8 +9,10 @@ #include "DependencyScannerImpl.h" #include "clang/Basic/DiagnosticFrontend.h" #include "clang/Basic/DiagnosticSerialization.h" +#include "clang/Driver/Driver.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h" +#include "llvm/TargetParser/Host.h" using namespace clang; using namespace tooling; @@ -334,6 +336,17 @@ class ScanningDependencyDirectivesGetter : public DependencyDirectivesGetter { }; } // namespace +std::unique_ptr +clang::tooling::dependencies::createDiagOptions( + const std::vector &CommandLine) { + std::vector CLI; + for (const std::string &Arg : CommandLine) + CLI.push_back(Arg.c_str()); + auto DiagOpts = CreateAndPopulateDiagOpts(CLI); + sanitizeDiagOpts(*DiagOpts); + return DiagOpts; +} + /// Sanitize diagnostic options for dependency scan. void clang::tooling::dependencies::sanitizeDiagOpts( DiagnosticOptions &DiagOpts) { @@ -356,43 +369,131 @@ void clang::tooling::dependencies::sanitizeDiagOpts( }); } -bool DependencyScanningAction::runInvocation( - std::shared_ptr Invocation, - IntrusiveRefCntPtr FS, - std::shared_ptr PCHContainerOps, - DiagnosticConsumer *DiagConsumer) { - // Making sure that we canonicalize the defines before we create the deep - // copy to avoid unnecessary variants in the scanner and in the resulting - // explicit command lines. - if (any(Service.getOptimizeArgs() & ScanningOptimizations::Macros)) - canonicalizeDefines(Invocation->getPreprocessorOpts()); +std::pair, std::unique_ptr> +clang::tooling::dependencies::buildCompilation( + ArrayRef ArgStrs, DiagnosticsEngine &Diags, + IntrusiveRefCntPtr FS) { + SmallVector Argv; + Argv.reserve(ArgStrs.size()); + for (const std::string &Arg : ArgStrs) + Argv.push_back(Arg.c_str()); + + std::unique_ptr Driver = std::make_unique( + Argv[0], llvm::sys::getDefaultTargetTriple(), Diags, + "clang LLVM compiler", FS); + Driver->setTitle("clang_based_tool"); + + llvm::BumpPtrAllocator Alloc; + bool CLMode = driver::IsClangCL( + driver::getDriverMode(Argv[0], ArrayRef(Argv).slice(1))); + + if (llvm::Error E = + driver::expandResponseFiles(Argv, CLMode, Alloc, FS.get())) { + Diags.Report(diag::err_drv_expand_response_file) + << llvm::toString(std::move(E)); + return std::make_pair(nullptr, nullptr); + } - // Make a deep copy of the original Clang invocation. - CompilerInvocation OriginalInvocation(*Invocation); + std::unique_ptr Compilation( + Driver->BuildCompilation(llvm::ArrayRef(Argv))); + if (!Compilation) + return std::make_pair(nullptr, nullptr); - if (Scanned) { - // Scanning runs once for the first -cc1 invocation in a chain of driver - // jobs. For any dependent jobs, reuse the scanning result and just - // update the LastCC1Arguments to correspond to the new invocation. - // FIXME: to support multi-arch builds, each arch requires a separate scan - setLastCC1Arguments(std::move(OriginalInvocation)); - return true; + if (Compilation->containsError()) + return std::make_pair(nullptr, nullptr); + + return std::make_pair(std::move(Driver), std::move(Compilation)); +} + +llvm::IntrusiveRefCntPtr +clang::tooling::dependencies::initVFSForTUBufferScanning( + llvm::IntrusiveRefCntPtr BaseFS, + std::optional> &ModifiedCommandLine, + const std::vector &CommandLine, StringRef WorkingDirectory, + std::optional TUBuffer) { + // Reset what might have been modified in the previous worker invocation. + BaseFS->setCurrentWorkingDirectory(WorkingDirectory); + + llvm::IntrusiveRefCntPtr ModifiedFS; + if (TUBuffer) { + auto OverlayFS = + llvm::makeIntrusiveRefCnt(BaseFS); + auto InMemoryFS = + llvm::makeIntrusiveRefCnt(); + InMemoryFS->setCurrentWorkingDirectory(WorkingDirectory); + auto InputPath = TUBuffer->getBufferIdentifier(); + InMemoryFS->addFile( + InputPath, 0, + llvm::MemoryBuffer::getMemBufferCopy(TUBuffer->getBuffer())); + llvm::IntrusiveRefCntPtr InMemoryOverlay = + InMemoryFS; + + OverlayFS->pushOverlay(InMemoryOverlay); + ModifiedFS = OverlayFS; + ModifiedCommandLine = CommandLine; + ModifiedCommandLine->emplace_back(InputPath); + + return ModifiedFS; } - Scanned = true; + return BaseFS; +} - // Create a compiler instance to handle the actual work. - auto ModCache = makeInProcessModuleCache(Service.getModuleCacheEntries()); - ScanInstanceStorage.emplace(std::move(Invocation), std::move(PCHContainerOps), - ModCache.get()); - CompilerInstance &ScanInstance = *ScanInstanceStorage; +llvm::IntrusiveRefCntPtr +clang::tooling::dependencies::initVFSForByNameScanning( + llvm::IntrusiveRefCntPtr BaseFS, + std::vector &CommandLine, StringRef WorkingDirectory, + StringRef FakeFileNamePrefix) { + // Reset what might have been modified in the previous worker invocation. + BaseFS->setCurrentWorkingDirectory(WorkingDirectory); + + // If we're scanning based on a module name alone, we don't expect the client + // to provide us with an input file. However, the driver really wants to have + // one. Let's just make it up to make the driver happy. + auto OverlayFS = + llvm::makeIntrusiveRefCnt(BaseFS); + auto InMemoryFS = llvm::makeIntrusiveRefCnt(); + InMemoryFS->setCurrentWorkingDirectory(WorkingDirectory); + SmallString<128> FakeInputPath; + // TODO: We should retry the creation if the path already exists. + llvm::sys::fs::createUniquePath(FakeFileNamePrefix + "-%%%%%%%%.input", + FakeInputPath, + /*MakeAbsolute=*/false); + InMemoryFS->addFile(FakeInputPath, 0, llvm::MemoryBuffer::getMemBuffer("")); + llvm::IntrusiveRefCntPtr InMemoryOverlay = InMemoryFS; + OverlayFS->pushOverlay(InMemoryOverlay); + + CommandLine.emplace_back(FakeInputPath); + + return OverlayFS; +} + +std::unique_ptr +clang::tooling::dependencies::createCompilerInvocation( + const std::vector &CommandLine, DiagnosticsEngine &Diags) { + llvm::opt::ArgStringList Argv; + for (const std::string &Str : ArrayRef(CommandLine).drop_front()) + Argv.push_back(Str.c_str()); + + auto Invocation = std::make_unique(); + if (!CompilerInvocation::CreateFromArgs(*Invocation, Argv, Diags)) { + // FIXME: Should we just go on like cc1_main does? + return nullptr; + } + return Invocation; +} + +bool clang::tooling::dependencies::initializeScanCompilerInstance( + CompilerInstance &ScanInstance, + IntrusiveRefCntPtr FS, + DiagnosticConsumer *DiagConsumer, DependencyScanningService &Service, + llvm::IntrusiveRefCntPtr DepFS) { ScanInstance.setBuildingModule(false); ScanInstance.createVirtualFileSystem(FS, DiagConsumer); // Create the compiler's actual diagnostics engine. sanitizeDiagOpts(ScanInstance.getDiagnosticOpts()); - assert(!DiagConsumerFinished && "attempt to reuse finished consumer"); ScanInstance.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false); if (!ScanInstance.hasDiagnostics()) return false; @@ -434,6 +535,26 @@ bool DependencyScanningAction::runInvocation( ScanInstance.createSourceManager(*FileMgr); + // Consider different header search and diagnostic options to create + // different modules. This avoids the unsound aliasing of module PCMs. + // + // TODO: Implement diagnostic bucketing to reduce the impact of strict + // context hashing. + ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true; + ScanInstance.getHeaderSearchOpts().ModulesSerializeOnlyPreprocessor = true; + ScanInstance.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true; + ScanInstance.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true; + ScanInstance.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings = true; + ScanInstance.getHeaderSearchOpts().ModulesForceValidateUserHeaders = false; + + // Avoid some checks and module map parsing when loading PCM files. + ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false; + + return true; +} + +llvm::SmallVector clang::tooling::dependencies::computeStableDirs( + CompilerInstance &ScanInstance) { // Create a collection of stable directories derived from the ScanInstance // for determining whether module dependencies would fully resolve from // those directories. @@ -441,7 +562,12 @@ bool DependencyScanningAction::runInvocation( const StringRef Sysroot = ScanInstance.getHeaderSearchOpts().Sysroot; if (!Sysroot.empty() && (llvm::sys::path::root_directory(Sysroot) != Sysroot)) StableDirs = {Sysroot, ScanInstance.getHeaderSearchOpts().ResourceDir}; + return StableDirs; +} +std::optional +clang::tooling::dependencies::computePrebuiltModulesASTMap( + CompilerInstance &ScanInstance, llvm::SmallVector &StableDirs) { // Store a mapping of prebuilt module files and their properties like header // search options. This will prevent the implicit build to create duplicate // modules and will force reuse of the existing prebuilt module files @@ -453,8 +579,18 @@ bool DependencyScanningAction::runInvocation( ScanInstance.getPreprocessorOpts().ImplicitPCHInclude, ScanInstance, ScanInstance.getHeaderSearchOpts().PrebuiltModuleFiles, PrebuiltModulesASTMap, ScanInstance.getDiagnostics(), StableDirs)) - return false; + return {}; + return PrebuiltModulesASTMap; +} + +void clang::tooling::dependencies::initializeModuleDepCollector( + CompilerInstance &ScanInstance, std::shared_ptr &MDC, + StringRef WorkingDirectory, DependencyConsumer &Consumer, + DependencyScanningService &Service, CompilerInvocation &Inv, + DependencyActionController &Controller, + PrebuiltModulesAttrsMap PrebuiltModulesASTMap, + llvm::SmallVector &StableDirs) { // Create the dependency collector that will collect the produced // dependencies. // @@ -463,6 +599,11 @@ bool DependencyScanningAction::runInvocation( // which ensures that the compiler won't create new dependency collectors, // and thus won't write out the extra '.d' files to disk. auto Opts = std::make_unique(); + + // We rely on the behaviour that that ScanInstance's Invocation instance's + // dependency output opts are cleared here. + // TODO: we will need to preserve and recover the original dependency output + // opts if we want to reuse ScanInstance. std::swap(*Opts, ScanInstance.getInvocation().getDependencyOutputOpts()); // We need at least one -MT equivalent for the generator of make dependency // files to work. @@ -480,26 +621,58 @@ bool DependencyScanningAction::runInvocation( case ScanningOutputFormat::P1689: case ScanningOutputFormat::Full: MDC = std::make_shared( - Service, std::move(Opts), ScanInstance, Consumer, Controller, - OriginalInvocation, std::move(PrebuiltModulesASTMap), StableDirs); + Service, std::move(Opts), ScanInstance, Consumer, Controller, Inv, + std::move(PrebuiltModulesASTMap), StableDirs); ScanInstance.addDependencyCollector(MDC); break; } +} - // Consider different header search and diagnostic options to create - // different modules. This avoids the unsound aliasing of module PCMs. - // - // TODO: Implement diagnostic bucketing to reduce the impact of strict - // context hashing. - ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true; - ScanInstance.getHeaderSearchOpts().ModulesSerializeOnlyPreprocessor = true; - ScanInstance.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true; - ScanInstance.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true; - ScanInstance.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings = true; - ScanInstance.getHeaderSearchOpts().ModulesForceValidateUserHeaders = false; +bool DependencyScanningAction::runInvocation( + std::unique_ptr Invocation, + IntrusiveRefCntPtr FS, + std::shared_ptr PCHContainerOps, + DiagnosticConsumer *DiagConsumer) { + // Making sure that we canonicalize the defines before we create the deep + // copy to avoid unnecessary variants in the scanner and in the resulting + // explicit command lines. + if (any(Service.getOptimizeArgs() & ScanningOptimizations::Macros)) + canonicalizeDefines(Invocation->getPreprocessorOpts()); - // Avoid some checks and module map parsing when loading PCM files. - ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false; + // Make a deep copy of the original Clang invocation. + CompilerInvocation OriginalInvocation(*Invocation); + + if (Scanned) { + // Scanning runs once for the first -cc1 invocation in a chain of driver + // jobs. For any dependent jobs, reuse the scanning result and just + // update the LastCC1Arguments to correspond to the new invocation. + // FIXME: to support multi-arch builds, each arch requires a separate scan + setLastCC1Arguments(std::move(OriginalInvocation)); + return true; + } + + Scanned = true; + + // Create a compiler instance to handle the actual work. + auto ModCache = makeInProcessModuleCache(Service.getModuleCacheEntries()); + ScanInstanceStorage.emplace(std::move(Invocation), std::move(PCHContainerOps), + ModCache.get()); + CompilerInstance &ScanInstance = *ScanInstanceStorage; + + assert(!DiagConsumerFinished && "attempt to reuse finished consumer"); + if (!initializeScanCompilerInstance(ScanInstance, FS, DiagConsumer, Service, + DepFS)) + return false; + + llvm::SmallVector StableDirs = computeStableDirs(ScanInstance); + auto MaybePrebuiltModulesASTMap = + computePrebuiltModulesASTMap(ScanInstance, StableDirs); + if (!MaybePrebuiltModulesASTMap) + return false; + + initializeModuleDepCollector(ScanInstance, MDC, WorkingDirectory, Consumer, + Service, OriginalInvocation, Controller, + *MaybePrebuiltModulesASTMap, StableDirs); std::unique_ptr Action; diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h index 32fbcfffde53c..c9779fe65e711 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h @@ -9,8 +9,10 @@ #ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNER_H #define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNER_H +#include "clang/Driver/Compilation.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInvocation.h" +#include "clang/Frontend/TextDiagnosticPrinter.h" #include "clang/Serialization/ObjectFilePCHContainerReader.h" #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h" #include "clang/Tooling/DependencyScanning/ModuleDepCollector.h" @@ -35,7 +37,7 @@ class DependencyScanningAction { : Service(Service), WorkingDirectory(WorkingDirectory), Consumer(Consumer), Controller(Controller), DepFS(std::move(DepFS)), ModuleName(ModuleName) {} - bool runInvocation(std::shared_ptr Invocation, + bool runInvocation(std::unique_ptr Invocation, IntrusiveRefCntPtr FS, std::shared_ptr PCHContainerOps, DiagnosticConsumer *DiagConsumer); @@ -73,8 +75,63 @@ class DependencyScanningAction { }; // Helper functions +std::unique_ptr +createDiagOptions(const std::vector &CommandLine); void sanitizeDiagOpts(DiagnosticOptions &DiagOpts); +struct TextDiagnosticsPrinterWithOutput { + std::string DiagnosticOutput; + llvm::raw_string_ostream DiagnosticsOS; + std::unique_ptr DiagOpts; + TextDiagnosticPrinter DiagPrinter; + + TextDiagnosticsPrinterWithOutput(const std::vector &CommandLine) + : DiagnosticsOS(DiagnosticOutput), + DiagOpts(createDiagOptions(CommandLine)), + DiagPrinter(DiagnosticsOS, *DiagOpts) {} +}; + +std::pair, std::unique_ptr> +buildCompilation(ArrayRef ArgStrs, DiagnosticsEngine &Diags, + IntrusiveRefCntPtr FS); + +std::unique_ptr +createCompilerInvocation(const std::vector &CommandLine, + DiagnosticsEngine &Diags); + +llvm::IntrusiveRefCntPtr initVFSForTUBufferScanning( + llvm::IntrusiveRefCntPtr BaseFS, + std::optional> &ModifiedCommandLine, + const std::vector &CommandLine, StringRef WorkingDirectory, + std::optional TUBuffer); + +llvm::IntrusiveRefCntPtr +initVFSForByNameScanning(llvm::IntrusiveRefCntPtr BaseFS, + std::vector &CommandLine, + StringRef WorkingDirectory, + StringRef FakeFileNamePrefix); + +bool initializeScanCompilerInstance( + CompilerInstance &ScanInstance, + IntrusiveRefCntPtr FS, + DiagnosticConsumer *DiagConsumer, DependencyScanningService &Service, + llvm::IntrusiveRefCntPtr DepFS); + +llvm::SmallVector computeStableDirs(CompilerInstance &ScanInstance); + +std::optional +computePrebuiltModulesASTMap(CompilerInstance &ScanInstance, + llvm::SmallVector &StableDirs); + +void initializeModuleDepCollector(CompilerInstance &ScanInstance, + std::shared_ptr &MDC, + StringRef WorkingDirectory, + DependencyConsumer &Consumer, + DependencyScanningService &Service, + CompilerInvocation &Inv, + DependencyActionController &Controller, + PrebuiltModulesAttrsMap PrebuiltModulesASTMap, + llvm::SmallVector &StableDirs); } // namespace dependencies } // namespace tooling } // namespace clang diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index 796e587ba9147..4a80189288d9c 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -63,32 +63,19 @@ DependencyScanningWorker::DependencyScanningWorker( } } -static std::unique_ptr -createDiagOptions(const std::vector &CommandLine) { - std::vector CLI; - for (const std::string &Arg : CommandLine) - CLI.push_back(Arg.c_str()); - auto DiagOpts = CreateAndPopulateDiagOpts(CLI); - sanitizeDiagOpts(*DiagOpts); - return DiagOpts; -} - llvm::Error DependencyScanningWorker::computeDependencies( StringRef WorkingDirectory, const std::vector &CommandLine, DependencyConsumer &Consumer, DependencyActionController &Controller, std::optional TUBuffer) { // Capture the emitted diagnostics and report them to the client // in the case of a failure. - std::string DiagnosticOutput; - llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput); - auto DiagOpts = createDiagOptions(CommandLine); - TextDiagnosticPrinter DiagPrinter(DiagnosticsOS, *DiagOpts); + TextDiagnosticsPrinterWithOutput DiagPrinterWithOS(CommandLine); if (computeDependencies(WorkingDirectory, CommandLine, Consumer, Controller, - DiagPrinter, TUBuffer)) + DiagPrinterWithOS.DiagPrinter, TUBuffer)) return llvm::Error::success(); - return llvm::make_error(DiagnosticsOS.str(), - llvm::inconvertibleErrorCode()); + return llvm::make_error( + DiagPrinterWithOS.DiagnosticsOS.str(), llvm::inconvertibleErrorCode()); } llvm::Error DependencyScanningWorker::computeDependencies( @@ -97,51 +84,24 @@ llvm::Error DependencyScanningWorker::computeDependencies( StringRef ModuleName) { // Capture the emitted diagnostics and report them to the client // in the case of a failure. - std::string DiagnosticOutput; - llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput); - auto DiagOpts = createDiagOptions(CommandLine); - TextDiagnosticPrinter DiagPrinter(DiagnosticsOS, *DiagOpts); + TextDiagnosticsPrinterWithOutput DiagPrinterWithOS(CommandLine); if (computeDependencies(WorkingDirectory, CommandLine, Consumer, Controller, - DiagPrinter, ModuleName)) + DiagPrinterWithOS.DiagPrinter, ModuleName)) return llvm::Error::success(); - return llvm::make_error(DiagnosticsOS.str(), - llvm::inconvertibleErrorCode()); + return llvm::make_error( + DiagPrinterWithOS.DiagnosticsOS.str(), llvm::inconvertibleErrorCode()); } static bool forEachDriverJob( ArrayRef ArgStrs, DiagnosticsEngine &Diags, IntrusiveRefCntPtr FS, llvm::function_ref Callback) { - SmallVector Argv; - Argv.reserve(ArgStrs.size()); - for (const std::string &Arg : ArgStrs) - Argv.push_back(Arg.c_str()); - - std::unique_ptr Driver = std::make_unique( - Argv[0], llvm::sys::getDefaultTargetTriple(), Diags, - "clang LLVM compiler", FS); - Driver->setTitle("clang_based_tool"); - - llvm::BumpPtrAllocator Alloc; - bool CLMode = driver::IsClangCL( - driver::getDriverMode(Argv[0], ArrayRef(Argv).slice(1))); - - if (llvm::Error E = - driver::expandResponseFiles(Argv, CLMode, Alloc, FS.get())) { - Diags.Report(diag::err_drv_expand_response_file) - << llvm::toString(std::move(E)); - return false; - } - - const std::unique_ptr Compilation( - Driver->BuildCompilation(llvm::ArrayRef(Argv))); + // Compilation owns a reference to the Driver, hence we need to + // keep the Driver alive when we use Compilation. + auto [Driver, Compilation] = buildCompilation(ArgStrs, Diags, FS); if (!Compilation) return false; - - if (Compilation->containsError()) - return false; - for (const driver::Command &Job : Compilation->getJobs()) { if (!Callback(Job)) return false; @@ -150,30 +110,21 @@ static bool forEachDriverJob( } static bool createAndRunToolInvocation( - std::vector CommandLine, DependencyScanningAction &Action, + const std::vector &CommandLine, + DependencyScanningAction &Action, IntrusiveRefCntPtr FS, std::shared_ptr &PCHContainerOps, DiagnosticsEngine &Diags, DependencyConsumer &Consumer) { - - // Save executable path before providing CommandLine to ToolInvocation - std::string Executable = CommandLine[0]; - - llvm::opt::ArgStringList Argv; - for (const std::string &Str : ArrayRef(CommandLine).drop_front()) - Argv.push_back(Str.c_str()); - - auto Invocation = std::make_shared(); - if (!CompilerInvocation::CreateFromArgs(*Invocation, Argv, Diags)) { - // FIXME: Should we just go on like cc1_main does? + auto Invocation = createCompilerInvocation(CommandLine, Diags); + if (!Invocation) return false; - } if (!Action.runInvocation(std::move(Invocation), std::move(FS), PCHContainerOps, Diags.getClient())) return false; std::vector Args = Action.takeLastCC1Arguments(); - Consumer.handleBuildCommand({std::move(Executable), std::move(Args)}); + Consumer.handleBuildCommand({CommandLine[0], std::move(Args)}); return true; } @@ -238,37 +189,11 @@ bool DependencyScanningWorker::computeDependencies( StringRef WorkingDirectory, const std::vector &CommandLine, DependencyConsumer &Consumer, DependencyActionController &Controller, DiagnosticConsumer &DC, std::optional TUBuffer) { - // Reset what might have been modified in the previous worker invocation. - BaseFS->setCurrentWorkingDirectory(WorkingDirectory); - std::optional> ModifiedCommandLine; - llvm::IntrusiveRefCntPtr ModifiedFS; - - // If we're scanning based on a module name alone, we don't expect the client - // to provide us with an input file. However, the driver really wants to have - // one. Let's just make it up to make the driver happy. - if (TUBuffer) { - auto OverlayFS = - llvm::makeIntrusiveRefCnt(BaseFS); - auto InMemoryFS = - llvm::makeIntrusiveRefCnt(); - InMemoryFS->setCurrentWorkingDirectory(WorkingDirectory); - auto InputPath = TUBuffer->getBufferIdentifier(); - InMemoryFS->addFile( - InputPath, 0, - llvm::MemoryBuffer::getMemBufferCopy(TUBuffer->getBuffer())); - llvm::IntrusiveRefCntPtr InMemoryOverlay = - InMemoryFS; - - OverlayFS->pushOverlay(InMemoryOverlay); - ModifiedFS = OverlayFS; - ModifiedCommandLine = CommandLine; - ModifiedCommandLine->emplace_back(InputPath); - } - + auto FinalFS = initVFSForTUBufferScanning( + BaseFS, ModifiedCommandLine, CommandLine, WorkingDirectory, TUBuffer); const std::vector &FinalCommandLine = ModifiedCommandLine ? *ModifiedCommandLine : CommandLine; - auto &FinalFS = ModifiedFS ? ModifiedFS : BaseFS; return scanDependencies(WorkingDirectory, FinalCommandLine, Consumer, Controller, DC, FinalFS, /*ModuleName=*/std::nullopt); @@ -278,26 +203,9 @@ bool DependencyScanningWorker::computeDependencies( StringRef WorkingDirectory, const std::vector &CommandLine, DependencyConsumer &Consumer, DependencyActionController &Controller, DiagnosticConsumer &DC, StringRef ModuleName) { - // Reset what might have been modified in the previous worker invocation. - BaseFS->setCurrentWorkingDirectory(WorkingDirectory); - - // If we're scanning based on a module name alone, we don't expect the client - // to provide us with an input file. However, the driver really wants to have - // one. Let's just make it up to make the driver happy. - auto OverlayFS = - llvm::makeIntrusiveRefCnt(BaseFS); - auto InMemoryFS = llvm::makeIntrusiveRefCnt(); - InMemoryFS->setCurrentWorkingDirectory(WorkingDirectory); - SmallString<128> FakeInputPath; - // TODO: We should retry the creation if the path already exists. - llvm::sys::fs::createUniquePath(ModuleName + "-%%%%%%%%.input", FakeInputPath, - /*MakeAbsolute=*/false); - InMemoryFS->addFile(FakeInputPath, 0, llvm::MemoryBuffer::getMemBuffer("")); - llvm::IntrusiveRefCntPtr InMemoryOverlay = InMemoryFS; - - OverlayFS->pushOverlay(InMemoryOverlay); auto ModifiedCommandLine = CommandLine; - ModifiedCommandLine.emplace_back(FakeInputPath); + auto OverlayFS = initVFSForByNameScanning(BaseFS, ModifiedCommandLine, + WorkingDirectory, ModuleName); return scanDependencies(WorkingDirectory, ModifiedCommandLine, Consumer, Controller, DC, OverlayFS, ModuleName); From e076b56b772f6b92a88dfb8a134079e64bc36bf9 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Tue, 30 Sep 2025 16:27:07 -0700 Subject: [PATCH 2/5] Addressing code review comments. --- .../Tooling/DependencyScanning/CMakeLists.txt | 1 - .../DependencyScannerImpl.cpp | 112 +++++++++--------- .../DependencyScannerImpl.h | 39 ++++-- .../DependencyScanningWorker.cpp | 72 ++++------- 4 files changed, 110 insertions(+), 114 deletions(-) diff --git a/clang/lib/Tooling/DependencyScanning/CMakeLists.txt b/clang/lib/Tooling/DependencyScanning/CMakeLists.txt index 53a2728bd5786..76bdc50097fff 100644 --- a/clang/lib/Tooling/DependencyScanning/CMakeLists.txt +++ b/clang/lib/Tooling/DependencyScanning/CMakeLists.txt @@ -24,6 +24,5 @@ add_clang_library(clangDependencyScanning clangFrontend clangLex clangSerialization - clangTooling ${LLVM_PTHREAD_LIB} ) diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp index 9a8fabaab42ec..bc1e817c936a9 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp @@ -334,22 +334,9 @@ class ScanningDependencyDirectivesGetter : public DependencyDirectivesGetter { return DepFS->getDirectiveTokens(File.getName()); } }; -} // namespace - -std::unique_ptr -clang::tooling::dependencies::createDiagOptions( - const std::vector &CommandLine) { - std::vector CLI; - for (const std::string &Arg : CommandLine) - CLI.push_back(Arg.c_str()); - auto DiagOpts = CreateAndPopulateDiagOpts(CLI); - sanitizeDiagOpts(*DiagOpts); - return DiagOpts; -} /// Sanitize diagnostic options for dependency scan. -void clang::tooling::dependencies::sanitizeDiagOpts( - DiagnosticOptions &DiagOpts) { +void sanitizeDiagOpts(DiagnosticOptions &DiagOpts) { // Don't print 'X warnings and Y errors generated'. DiagOpts.ShowCarets = false; // Don't write out diagnostic file. @@ -368,6 +355,31 @@ void clang::tooling::dependencies::sanitizeDiagOpts( .Default(true); }); } +} // namespace + +std::unique_ptr +clang::tooling::dependencies::createDiagOptions( + const std::vector &CommandLine) { + std::vector CLI; + for (const std::string &Arg : CommandLine) + CLI.push_back(Arg.c_str()); + auto DiagOpts = CreateAndPopulateDiagOpts(CLI); + sanitizeDiagOpts(*DiagOpts); + return DiagOpts; +} + +clang::tooling::dependencies::DignosticsEngineWithCCommandLineAndDiagOpts:: + DignosticsEngineWithCCommandLineAndDiagOpts( + const std::vector CommandLine, + IntrusiveRefCntPtr FS, DiagnosticConsumer &DC) + : CCommandLine(CommandLine.size(), nullptr) { + llvm::transform(CommandLine, CCommandLine.begin(), + [](const std::string &Str) { return Str.c_str(); }); + DiagOpts = CreateAndPopulateDiagOpts(CCommandLine); + sanitizeDiagOpts(*DiagOpts); + DiagEngine = CompilerInstance::createDiagnostics(*FS, *DiagOpts, &DC, + /*ShouldOwnClient=*/false); +} std::pair, std::unique_ptr> clang::tooling::dependencies::buildCompilation( @@ -395,7 +407,7 @@ clang::tooling::dependencies::buildCompilation( } std::unique_ptr Compilation( - Driver->BuildCompilation(llvm::ArrayRef(Argv))); + Driver->BuildCompilation(Argv)); if (!Compilation) return std::make_pair(nullptr, nullptr); @@ -405,45 +417,37 @@ clang::tooling::dependencies::buildCompilation( return std::make_pair(std::move(Driver), std::move(Compilation)); } -llvm::IntrusiveRefCntPtr -clang::tooling::dependencies::initVFSForTUBufferScanning( - llvm::IntrusiveRefCntPtr BaseFS, - std::optional> &ModifiedCommandLine, +std::pair, std::vector> +clang::tooling::dependencies::initVFSForTUBuferScanning( + IntrusiveRefCntPtr BaseFS, const std::vector &CommandLine, StringRef WorkingDirectory, - std::optional TUBuffer) { + llvm::MemoryBufferRef TUBuffer) { // Reset what might have been modified in the previous worker invocation. BaseFS->setCurrentWorkingDirectory(WorkingDirectory); - llvm::IntrusiveRefCntPtr ModifiedFS; - if (TUBuffer) { - auto OverlayFS = - llvm::makeIntrusiveRefCnt(BaseFS); - auto InMemoryFS = - llvm::makeIntrusiveRefCnt(); - InMemoryFS->setCurrentWorkingDirectory(WorkingDirectory); - auto InputPath = TUBuffer->getBufferIdentifier(); - InMemoryFS->addFile( - InputPath, 0, - llvm::MemoryBuffer::getMemBufferCopy(TUBuffer->getBuffer())); - llvm::IntrusiveRefCntPtr InMemoryOverlay = - InMemoryFS; - - OverlayFS->pushOverlay(InMemoryOverlay); - ModifiedFS = OverlayFS; - ModifiedCommandLine = CommandLine; - ModifiedCommandLine->emplace_back(InputPath); - - return ModifiedFS; - } + IntrusiveRefCntPtr ModifiedFS; + auto OverlayFS = + llvm::makeIntrusiveRefCnt(BaseFS); + auto InMemoryFS = llvm::makeIntrusiveRefCnt(); + InMemoryFS->setCurrentWorkingDirectory(WorkingDirectory); + auto InputPath = TUBuffer.getBufferIdentifier(); + InMemoryFS->addFile( + InputPath, 0, llvm::MemoryBuffer::getMemBufferCopy(TUBuffer.getBuffer())); + IntrusiveRefCntPtr InMemoryOverlay = InMemoryFS; + + OverlayFS->pushOverlay(InMemoryOverlay); + ModifiedFS = OverlayFS; + auto ModifiedCommandLine = CommandLine; + ModifiedCommandLine.emplace_back(InputPath); - return BaseFS; + return std::make_pair(ModifiedFS, ModifiedCommandLine); } -llvm::IntrusiveRefCntPtr +std::pair, std::vector> clang::tooling::dependencies::initVFSForByNameScanning( - llvm::IntrusiveRefCntPtr BaseFS, - std::vector &CommandLine, StringRef WorkingDirectory, - StringRef FakeFileNamePrefix) { + IntrusiveRefCntPtr BaseFS, + const std::vector &CommandLine, StringRef WorkingDirectory, + StringRef ModuleName) { // Reset what might have been modified in the previous worker invocation. BaseFS->setCurrentWorkingDirectory(WorkingDirectory); @@ -456,16 +460,16 @@ clang::tooling::dependencies::initVFSForByNameScanning( InMemoryFS->setCurrentWorkingDirectory(WorkingDirectory); SmallString<128> FakeInputPath; // TODO: We should retry the creation if the path already exists. - llvm::sys::fs::createUniquePath(FakeFileNamePrefix + "-%%%%%%%%.input", - FakeInputPath, + llvm::sys::fs::createUniquePath(ModuleName + "-%%%%%%%%.input", FakeInputPath, /*MakeAbsolute=*/false); InMemoryFS->addFile(FakeInputPath, 0, llvm::MemoryBuffer::getMemBuffer("")); - llvm::IntrusiveRefCntPtr InMemoryOverlay = InMemoryFS; + IntrusiveRefCntPtr InMemoryOverlay = InMemoryFS; OverlayFS->pushOverlay(InMemoryOverlay); - CommandLine.emplace_back(FakeInputPath); + auto ModifiedCommandLine = CommandLine; + ModifiedCommandLine.emplace_back(FakeInputPath); - return OverlayFS; + return std::make_pair(OverlayFS, ModifiedCommandLine); } std::unique_ptr @@ -487,7 +491,7 @@ bool clang::tooling::dependencies::initializeScanCompilerInstance( CompilerInstance &ScanInstance, IntrusiveRefCntPtr FS, DiagnosticConsumer *DiagConsumer, DependencyScanningService &Service, - llvm::IntrusiveRefCntPtr DepFS) { + IntrusiveRefCntPtr DepFS) { ScanInstance.setBuildingModule(false); ScanInstance.createVirtualFileSystem(FS, DiagConsumer); @@ -554,7 +558,7 @@ bool clang::tooling::dependencies::initializeScanCompilerInstance( } llvm::SmallVector clang::tooling::dependencies::computeStableDirs( - CompilerInstance &ScanInstance) { + const CompilerInstance &ScanInstance) { // Create a collection of stable directories derived from the ScanInstance // for determining whether module dependencies would fully resolve from // those directories. @@ -600,7 +604,7 @@ void clang::tooling::dependencies::initializeModuleDepCollector( // and thus won't write out the extra '.d' files to disk. auto Opts = std::make_unique(); - // We rely on the behaviour that that ScanInstance's Invocation instance's + // We rely on the behaviour that the ScanInstance's Invocation instance's // dependency output opts are cleared here. // TODO: we will need to preserve and recover the original dependency output // opts if we want to reuse ScanInstance. diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h index c9779fe65e711..44015490a036a 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h @@ -74,15 +74,28 @@ class DependencyScanningAction { bool DiagConsumerFinished = false; }; -// Helper functions +// Helper functions and data types. std::unique_ptr createDiagOptions(const std::vector &CommandLine); -void sanitizeDiagOpts(DiagnosticOptions &DiagOpts); + +struct DignosticsEngineWithCCommandLineAndDiagOpts { + // We need to bound the lifetime of the CCommandLine and the DiagOpts + // used to create the DiganosticsEngine with the DiagnosticsEngine itself. + std::vector CCommandLine; + std::unique_ptr DiagOpts; + IntrusiveRefCntPtr DiagEngine; + + DignosticsEngineWithCCommandLineAndDiagOpts( + const std::vector CommandLine, + IntrusiveRefCntPtr FS, DiagnosticConsumer &DC); +}; struct TextDiagnosticsPrinterWithOutput { + // We need to bound the lifetime of the data that supports the DiagPrinter + // with it together so they have the same lifetime. std::string DiagnosticOutput; llvm::raw_string_ostream DiagnosticsOS; - std::unique_ptr DiagOpts; + std::unique_ptr DiagOpts; TextDiagnosticPrinter DiagPrinter; TextDiagnosticsPrinterWithOutput(const std::vector &CommandLine) @@ -99,17 +112,16 @@ std::unique_ptr createCompilerInvocation(const std::vector &CommandLine, DiagnosticsEngine &Diags); -llvm::IntrusiveRefCntPtr initVFSForTUBufferScanning( - llvm::IntrusiveRefCntPtr BaseFS, - std::optional> &ModifiedCommandLine, - const std::vector &CommandLine, StringRef WorkingDirectory, - std::optional TUBuffer); +std::pair, std::vector> +initVFSForTUBuferScanning(IntrusiveRefCntPtr BaseFS, + const std::vector &CommandLine, + StringRef WorkingDirectory, + llvm::MemoryBufferRef TUBuffer); -llvm::IntrusiveRefCntPtr +std::pair, std::vector> initVFSForByNameScanning(llvm::IntrusiveRefCntPtr BaseFS, - std::vector &CommandLine, - StringRef WorkingDirectory, - StringRef FakeFileNamePrefix); + const std::vector &CommandLine, + StringRef WorkingDirectory, StringRef ModuleName); bool initializeScanCompilerInstance( CompilerInstance &ScanInstance, @@ -117,7 +129,8 @@ bool initializeScanCompilerInstance( DiagnosticConsumer *DiagConsumer, DependencyScanningService &Service, llvm::IntrusiveRefCntPtr DepFS); -llvm::SmallVector computeStableDirs(CompilerInstance &ScanInstance); +llvm::SmallVector +computeStableDirs(const CompilerInstance &ScanInstance); std::optional computePrebuiltModulesASTMap(CompilerInstance &ScanInstance, diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index 4a80189288d9c..5dc174f88f79f 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -8,29 +8,9 @@ #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h" #include "DependencyScannerImpl.h" -#include "clang/Basic/DiagnosticDriver.h" #include "clang/Basic/DiagnosticFrontend.h" -#include "clang/Basic/DiagnosticSerialization.h" -#include "clang/Driver/Compilation.h" #include "clang/Driver/Driver.h" -#include "clang/Driver/Job.h" #include "clang/Driver/Tool.h" -#include "clang/Frontend/CompilerInstance.h" -#include "clang/Frontend/CompilerInvocation.h" -#include "clang/Frontend/FrontendActions.h" -#include "clang/Frontend/TextDiagnosticPrinter.h" -#include "clang/Frontend/Utils.h" -#include "clang/Lex/PreprocessorOptions.h" -#include "clang/Serialization/ObjectFilePCHContainerReader.h" -#include "clang/Tooling/DependencyScanning/DependencyScanningService.h" -#include "clang/Tooling/DependencyScanning/InProcessModuleCache.h" -#include "clang/Tooling/DependencyScanning/ModuleDepCollector.h" -#include "llvm/ADT/IntrusiveRefCntPtr.h" -#include "llvm/Support/Allocator.h" -#include "llvm/Support/Error.h" -#include "llvm/Support/MemoryBuffer.h" -#include "llvm/TargetParser/Host.h" -#include using namespace clang; using namespace tooling; @@ -97,7 +77,7 @@ static bool forEachDriverJob( ArrayRef ArgStrs, DiagnosticsEngine &Diags, IntrusiveRefCntPtr FS, llvm::function_ref Callback) { - // Compilation owns a reference to the Driver, hence we need to + // Compilation holds a non-owning a reference to the Driver, hence we need to // keep the Driver alive when we use Compilation. auto [Driver, Compilation] = buildCompilation(ArgStrs, Diags, FS); if (!Compilation) @@ -133,24 +113,20 @@ bool DependencyScanningWorker::scanDependencies( DependencyConsumer &Consumer, DependencyActionController &Controller, DiagnosticConsumer &DC, llvm::IntrusiveRefCntPtr FS, std::optional ModuleName) { - std::vector CCommandLine(CommandLine.size(), nullptr); - llvm::transform(CommandLine, CCommandLine.begin(), - [](const std::string &Str) { return Str.c_str(); }); - auto DiagOpts = CreateAndPopulateDiagOpts(CCommandLine); - sanitizeDiagOpts(*DiagOpts); - auto Diags = CompilerInstance::createDiagnostics(*FS, *DiagOpts, &DC, - /*ShouldOwnClient=*/false); - + DignosticsEngineWithCCommandLineAndDiagOpts DiagEngineWithCmdAndOpts( + CommandLine, FS, DC); DependencyScanningAction Action(Service, WorkingDirectory, Consumer, Controller, DepFS, ModuleName); bool Success = false; if (CommandLine[1] == "-cc1") { - Success = createAndRunToolInvocation(CommandLine, Action, FS, - PCHContainerOps, *Diags, Consumer); + Success = createAndRunToolInvocation( + CommandLine, Action, FS, PCHContainerOps, + *DiagEngineWithCmdAndOpts.DiagEngine, Consumer); } else { Success = forEachDriverJob( - CommandLine, *Diags, FS, [&](const driver::Command &Cmd) { + CommandLine, *DiagEngineWithCmdAndOpts.DiagEngine, FS, + [&](const driver::Command &Cmd) { if (StringRef(Cmd.getCreator().getName()) != "clang") { // Non-clang command. Just pass through to the dependency // consumer. @@ -169,13 +145,15 @@ bool DependencyScanningWorker::scanDependencies( // system to ensure that any file system requests that // are made by the driver do not go through the // dependency scanning filesystem. - return createAndRunToolInvocation(std::move(Argv), Action, FS, - PCHContainerOps, *Diags, Consumer); + return createAndRunToolInvocation( + std::move(Argv), Action, FS, PCHContainerOps, + *DiagEngineWithCmdAndOpts.DiagEngine, Consumer); }); } if (Success && !Action.hasScanned()) - Diags->Report(diag::err_fe_expected_compiler_job) + DiagEngineWithCmdAndOpts.DiagEngine->Report( + diag::err_fe_expected_compiler_job) << llvm::join(CommandLine, " "); // Ensure finish() is called even if we never reached ExecuteAction(). @@ -189,23 +167,25 @@ bool DependencyScanningWorker::computeDependencies( StringRef WorkingDirectory, const std::vector &CommandLine, DependencyConsumer &Consumer, DependencyActionController &Controller, DiagnosticConsumer &DC, std::optional TUBuffer) { - std::optional> ModifiedCommandLine; - auto FinalFS = initVFSForTUBufferScanning( - BaseFS, ModifiedCommandLine, CommandLine, WorkingDirectory, TUBuffer); - const std::vector &FinalCommandLine = - ModifiedCommandLine ? *ModifiedCommandLine : CommandLine; - - return scanDependencies(WorkingDirectory, FinalCommandLine, Consumer, - Controller, DC, FinalFS, /*ModuleName=*/std::nullopt); + if (TUBuffer) { + auto [FinalFS, FinalCommandLine] = initVFSForTUBuferScanning( + BaseFS, CommandLine, WorkingDirectory, *TUBuffer); + return scanDependencies(WorkingDirectory, FinalCommandLine, Consumer, + Controller, DC, FinalFS, + /*ModuleName=*/std::nullopt); + } else { + BaseFS->setCurrentWorkingDirectory(WorkingDirectory); + return scanDependencies(WorkingDirectory, CommandLine, Consumer, Controller, + DC, BaseFS, /*ModuleName=*/std::nullopt); + } } bool DependencyScanningWorker::computeDependencies( StringRef WorkingDirectory, const std::vector &CommandLine, DependencyConsumer &Consumer, DependencyActionController &Controller, DiagnosticConsumer &DC, StringRef ModuleName) { - auto ModifiedCommandLine = CommandLine; - auto OverlayFS = initVFSForByNameScanning(BaseFS, ModifiedCommandLine, - WorkingDirectory, ModuleName); + auto [OverlayFS, ModifiedCommandLine] = initVFSForByNameScanning( + BaseFS, CommandLine, WorkingDirectory, ModuleName); return scanDependencies(WorkingDirectory, ModifiedCommandLine, Consumer, Controller, DC, OverlayFS, ModuleName); From b7b08e861de9d312c419468f096a775ac76a8af1 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Wed, 1 Oct 2025 16:06:24 -0700 Subject: [PATCH 3/5] Address code review comments. --- .../DependencyScannerImpl.cpp | 128 +++++++++--------- .../DependencyScannerImpl.h | 58 ++++---- .../DependencyScanningWorker.cpp | 3 +- 3 files changed, 96 insertions(+), 93 deletions(-) diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp index bc1e817c936a9..9797523a1c670 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp @@ -357,9 +357,9 @@ void sanitizeDiagOpts(DiagnosticOptions &DiagOpts) { } } // namespace +namespace clang::tooling::dependencies { std::unique_ptr -clang::tooling::dependencies::createDiagOptions( - const std::vector &CommandLine) { +createDiagOptions(ArrayRef CommandLine) { std::vector CLI; for (const std::string &Arg : CommandLine) CLI.push_back(Arg.c_str()); @@ -368,11 +368,10 @@ clang::tooling::dependencies::createDiagOptions( return DiagOpts; } -clang::tooling::dependencies::DignosticsEngineWithCCommandLineAndDiagOpts:: - DignosticsEngineWithCCommandLineAndDiagOpts( - const std::vector CommandLine, - IntrusiveRefCntPtr FS, DiagnosticConsumer &DC) - : CCommandLine(CommandLine.size(), nullptr) { +DignosticsEngineWithDiagOpts::DignosticsEngineWithDiagOpts( + ArrayRef CommandLine, + IntrusiveRefCntPtr FS, DiagnosticConsumer &DC) { + std::vector CCommandLine(CommandLine.size(), nullptr); llvm::transform(CommandLine, CCommandLine.begin(), [](const std::string &Str) { return Str.c_str(); }); DiagOpts = CreateAndPopulateDiagOpts(CCommandLine); @@ -382,9 +381,8 @@ clang::tooling::dependencies::DignosticsEngineWithCCommandLineAndDiagOpts:: } std::pair, std::unique_ptr> -clang::tooling::dependencies::buildCompilation( - ArrayRef ArgStrs, DiagnosticsEngine &Diags, - IntrusiveRefCntPtr FS) { +buildCompilation(ArrayRef ArgStrs, DiagnosticsEngine &Diags, + IntrusiveRefCntPtr FS) { SmallVector Argv; Argv.reserve(ArgStrs.size()); for (const std::string &Arg : ArgStrs) @@ -417,11 +415,26 @@ clang::tooling::dependencies::buildCompilation( return std::make_pair(std::move(Driver), std::move(Compilation)); } +std::unique_ptr +createCompilerInvocation(ArrayRef CommandLine, + DiagnosticsEngine &Diags) { + llvm::opt::ArgStringList Argv; + for (const std::string &Str : ArrayRef(CommandLine).drop_front()) + Argv.push_back(Str.c_str()); + + auto Invocation = std::make_unique(); + if (!CompilerInvocation::CreateFromArgs(*Invocation, Argv, Diags)) { + // FIXME: Should we just go on like cc1_main does? + return nullptr; + } + return Invocation; +} + std::pair, std::vector> -clang::tooling::dependencies::initVFSForTUBuferScanning( - IntrusiveRefCntPtr BaseFS, - const std::vector &CommandLine, StringRef WorkingDirectory, - llvm::MemoryBufferRef TUBuffer) { +initVFSForTUBuferScanning(IntrusiveRefCntPtr BaseFS, + ArrayRef CommandLine, + StringRef WorkingDirectory, + llvm::MemoryBufferRef TUBuffer) { // Reset what might have been modified in the previous worker invocation. BaseFS->setCurrentWorkingDirectory(WorkingDirectory); @@ -437,17 +450,16 @@ clang::tooling::dependencies::initVFSForTUBuferScanning( OverlayFS->pushOverlay(InMemoryOverlay); ModifiedFS = OverlayFS; - auto ModifiedCommandLine = CommandLine; + std::vector ModifiedCommandLine(CommandLine); ModifiedCommandLine.emplace_back(InputPath); return std::make_pair(ModifiedFS, ModifiedCommandLine); } std::pair, std::vector> -clang::tooling::dependencies::initVFSForByNameScanning( - IntrusiveRefCntPtr BaseFS, - const std::vector &CommandLine, StringRef WorkingDirectory, - StringRef ModuleName) { +initVFSForByNameScanning(IntrusiveRefCntPtr BaseFS, + ArrayRef CommandLine, + StringRef WorkingDirectory, StringRef ModuleName) { // Reset what might have been modified in the previous worker invocation. BaseFS->setCurrentWorkingDirectory(WorkingDirectory); @@ -466,28 +478,13 @@ clang::tooling::dependencies::initVFSForByNameScanning( IntrusiveRefCntPtr InMemoryOverlay = InMemoryFS; OverlayFS->pushOverlay(InMemoryOverlay); - auto ModifiedCommandLine = CommandLine; + std::vector ModifiedCommandLine(CommandLine); ModifiedCommandLine.emplace_back(FakeInputPath); return std::make_pair(OverlayFS, ModifiedCommandLine); } -std::unique_ptr -clang::tooling::dependencies::createCompilerInvocation( - const std::vector &CommandLine, DiagnosticsEngine &Diags) { - llvm::opt::ArgStringList Argv; - for (const std::string &Str : ArrayRef(CommandLine).drop_front()) - Argv.push_back(Str.c_str()); - - auto Invocation = std::make_unique(); - if (!CompilerInvocation::CreateFromArgs(*Invocation, Argv, Diags)) { - // FIXME: Should we just go on like cc1_main does? - return nullptr; - } - return Invocation; -} - -bool clang::tooling::dependencies::initializeScanCompilerInstance( +bool initializeScanCompilerInstance( CompilerInstance &ScanInstance, IntrusiveRefCntPtr FS, DiagnosticConsumer *DiagConsumer, DependencyScanningService &Service, @@ -557,8 +554,8 @@ bool clang::tooling::dependencies::initializeScanCompilerInstance( return true; } -llvm::SmallVector clang::tooling::dependencies::computeStableDirs( - const CompilerInstance &ScanInstance) { +llvm::SmallVector +getInitialStableDirs(const CompilerInstance &ScanInstance) { // Create a collection of stable directories derived from the ScanInstance // for determining whether module dependencies would fully resolve from // those directories. @@ -570,8 +567,8 @@ llvm::SmallVector clang::tooling::dependencies::computeStableDirs( } std::optional -clang::tooling::dependencies::computePrebuiltModulesASTMap( - CompilerInstance &ScanInstance, llvm::SmallVector &StableDirs) { +computePrebuiltModulesASTMap(CompilerInstance &ScanInstance, + llvm::SmallVector &StableDirs) { // Store a mapping of prebuilt module files and their properties like header // search options. This will prevent the implicit build to create duplicate // modules and will force reuse of the existing prebuilt module files @@ -588,34 +585,36 @@ clang::tooling::dependencies::computePrebuiltModulesASTMap( return PrebuiltModulesASTMap; } -void clang::tooling::dependencies::initializeModuleDepCollector( - CompilerInstance &ScanInstance, std::shared_ptr &MDC, - StringRef WorkingDirectory, DependencyConsumer &Consumer, - DependencyScanningService &Service, CompilerInvocation &Inv, - DependencyActionController &Controller, - PrebuiltModulesAttrsMap PrebuiltModulesASTMap, - llvm::SmallVector &StableDirs) { - // Create the dependency collector that will collect the produced - // dependencies. - // - // This also moves the existing dependency output options from the +std::unique_ptr +getDependencyOutputOptions(CompilerInstance &ScanInstance) { + // This function moves the existing dependency output options from the // invocation to the collector. The options in the invocation are reset, // which ensures that the compiler won't create new dependency collectors, // and thus won't write out the extra '.d' files to disk. auto Opts = std::make_unique(); - - // We rely on the behaviour that the ScanInstance's Invocation instance's - // dependency output opts are cleared here. - // TODO: we will need to preserve and recover the original dependency output - // opts if we want to reuse ScanInstance. std::swap(*Opts, ScanInstance.getInvocation().getDependencyOutputOpts()); - // We need at least one -MT equivalent for the generator of make dependency - // files to work. if (Opts->Targets.empty()) Opts->Targets = {deduceDepTarget(ScanInstance.getFrontendOpts().OutputFile, ScanInstance.getFrontendOpts().Inputs)}; Opts->IncludeSystemHeaders = true; + return Opts; +} + +std::shared_ptr initializeScanInstanceDependencyCollector( + CompilerInstance &ScanInstance, + const DependencyOutputOptions &DepOutputOpts, StringRef WorkingDirectory, + DependencyConsumer &Consumer, DependencyScanningService &Service, + CompilerInvocation &Inv, DependencyActionController &Controller, + PrebuiltModulesAttrsMap PrebuiltModulesASTMap, + llvm::SmallVector &StableDirs) { + // Create the dependency collector that will collect the produced + // dependencies. May return the created ModuleDepCollector depending + // on the scanning format. + + auto Opts = std::make_unique(DepOutputOpts); + + std::shared_ptr MDC; switch (Service.getFormat()) { case ScanningOutputFormat::Make: ScanInstance.addDependencyCollector( @@ -630,7 +629,10 @@ void clang::tooling::dependencies::initializeModuleDepCollector( ScanInstance.addDependencyCollector(MDC); break; } + + return MDC; } +} // namespace clang::tooling::dependencies bool DependencyScanningAction::runInvocation( std::unique_ptr Invocation, @@ -668,15 +670,17 @@ bool DependencyScanningAction::runInvocation( DepFS)) return false; - llvm::SmallVector StableDirs = computeStableDirs(ScanInstance); + llvm::SmallVector StableDirs = getInitialStableDirs(ScanInstance); auto MaybePrebuiltModulesASTMap = computePrebuiltModulesASTMap(ScanInstance, StableDirs); if (!MaybePrebuiltModulesASTMap) return false; - initializeModuleDepCollector(ScanInstance, MDC, WorkingDirectory, Consumer, - Service, OriginalInvocation, Controller, - *MaybePrebuiltModulesASTMap, StableDirs); + auto DepOutputOpts = getDependencyOutputOptions(ScanInstance); + + MDC = initializeScanInstanceDependencyCollector( + ScanInstance, *DepOutputOpts, WorkingDirectory, Consumer, Service, + OriginalInvocation, Controller, *MaybePrebuiltModulesASTMap, StableDirs); std::unique_ptr Action; diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h index 44015490a036a..9b79aa11f08b6 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h @@ -32,7 +32,7 @@ class DependencyScanningAction { DependencyScanningAction( DependencyScanningService &Service, StringRef WorkingDirectory, DependencyConsumer &Consumer, DependencyActionController &Controller, - llvm::IntrusiveRefCntPtr DepFS, + IntrusiveRefCntPtr DepFS, std::optional ModuleName = std::nullopt) : Service(Service), WorkingDirectory(WorkingDirectory), Consumer(Consumer), Controller(Controller), DepFS(std::move(DepFS)), @@ -65,7 +65,7 @@ class DependencyScanningAction { StringRef WorkingDirectory; DependencyConsumer &Consumer; DependencyActionController &Controller; - llvm::IntrusiveRefCntPtr DepFS; + IntrusiveRefCntPtr DepFS; std::optional ModuleName; std::optional ScanInstanceStorage; std::shared_ptr MDC; @@ -76,18 +76,17 @@ class DependencyScanningAction { // Helper functions and data types. std::unique_ptr -createDiagOptions(const std::vector &CommandLine); +createDiagOptions(ArrayRef CommandLine); -struct DignosticsEngineWithCCommandLineAndDiagOpts { - // We need to bound the lifetime of the CCommandLine and the DiagOpts - // used to create the DiganosticsEngine with the DiagnosticsEngine itself. - std::vector CCommandLine; +struct DignosticsEngineWithDiagOpts { + // We need to bound the lifetime of the DiagOpts used to create the + // DiganosticsEngine with the DiagnosticsEngine itself. std::unique_ptr DiagOpts; IntrusiveRefCntPtr DiagEngine; - DignosticsEngineWithCCommandLineAndDiagOpts( - const std::vector CommandLine, - IntrusiveRefCntPtr FS, DiagnosticConsumer &DC); + DignosticsEngineWithDiagOpts(ArrayRef CommandLine, + IntrusiveRefCntPtr FS, + DiagnosticConsumer &DC); }; struct TextDiagnosticsPrinterWithOutput { @@ -98,7 +97,7 @@ struct TextDiagnosticsPrinterWithOutput { std::unique_ptr DiagOpts; TextDiagnosticPrinter DiagPrinter; - TextDiagnosticsPrinterWithOutput(const std::vector &CommandLine) + TextDiagnosticsPrinterWithOutput(ArrayRef CommandLine) : DiagnosticsOS(DiagnosticOutput), DiagOpts(createDiagOptions(CommandLine)), DiagPrinter(DiagnosticsOS, *DiagOpts) {} @@ -109,42 +108,43 @@ buildCompilation(ArrayRef ArgStrs, DiagnosticsEngine &Diags, IntrusiveRefCntPtr FS); std::unique_ptr -createCompilerInvocation(const std::vector &CommandLine, +createCompilerInvocation(ArrayRef CommandLine, DiagnosticsEngine &Diags); std::pair, std::vector> initVFSForTUBuferScanning(IntrusiveRefCntPtr BaseFS, - const std::vector &CommandLine, + ArrayRef CommandLine, StringRef WorkingDirectory, llvm::MemoryBufferRef TUBuffer); std::pair, std::vector> -initVFSForByNameScanning(llvm::IntrusiveRefCntPtr BaseFS, - const std::vector &CommandLine, +initVFSForByNameScanning(IntrusiveRefCntPtr BaseFS, + ArrayRef CommandLine, StringRef WorkingDirectory, StringRef ModuleName); bool initializeScanCompilerInstance( CompilerInstance &ScanInstance, IntrusiveRefCntPtr FS, DiagnosticConsumer *DiagConsumer, DependencyScanningService &Service, - llvm::IntrusiveRefCntPtr DepFS); + IntrusiveRefCntPtr DepFS); -llvm::SmallVector -computeStableDirs(const CompilerInstance &ScanInstance); +SmallVector +getInitialStableDirs(const CompilerInstance &ScanInstance); std::optional computePrebuiltModulesASTMap(CompilerInstance &ScanInstance, - llvm::SmallVector &StableDirs); - -void initializeModuleDepCollector(CompilerInstance &ScanInstance, - std::shared_ptr &MDC, - StringRef WorkingDirectory, - DependencyConsumer &Consumer, - DependencyScanningService &Service, - CompilerInvocation &Inv, - DependencyActionController &Controller, - PrebuiltModulesAttrsMap PrebuiltModulesASTMap, - llvm::SmallVector &StableDirs); + SmallVector &StableDirs); + +std::unique_ptr +getDependencyOutputOptions(CompilerInstance &ScanInstance); + +std::shared_ptr initializeScanInstanceDependencyCollector( + CompilerInstance &ScanInstance, + const DependencyOutputOptions &DepOutputOpts, StringRef WorkingDirectory, + DependencyConsumer &Consumer, DependencyScanningService &Service, + CompilerInvocation &Inv, DependencyActionController &Controller, + PrebuiltModulesAttrsMap PrebuiltModulesASTMap, + llvm::SmallVector &StableDirs); } // namespace dependencies } // namespace tooling } // namespace clang diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index 5dc174f88f79f..95154212603ac 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -113,8 +113,7 @@ bool DependencyScanningWorker::scanDependencies( DependencyConsumer &Consumer, DependencyActionController &Controller, DiagnosticConsumer &DC, llvm::IntrusiveRefCntPtr FS, std::optional ModuleName) { - DignosticsEngineWithCCommandLineAndDiagOpts DiagEngineWithCmdAndOpts( - CommandLine, FS, DC); + DignosticsEngineWithDiagOpts DiagEngineWithCmdAndOpts(CommandLine, FS, DC); DependencyScanningAction Action(Service, WorkingDirectory, Consumer, Controller, DepFS, ModuleName); From de2358517d5c2b88a99cc9aeaa2046f31b363096 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Thu, 2 Oct 2025 11:23:09 -0700 Subject: [PATCH 4/5] Address code review comments. --- .../DependencyScannerImpl.cpp | 31 +++++++++---------- .../DependencyScannerImpl.h | 12 ++++--- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp index 9797523a1c670..2652edd2c0b51 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp @@ -586,12 +586,15 @@ computePrebuiltModulesASTMap(CompilerInstance &ScanInstance, } std::unique_ptr -getDependencyOutputOptions(CompilerInstance &ScanInstance) { +takeDependencyOutputOptionsFrom(CompilerInstance &ScanInstance) { // This function moves the existing dependency output options from the // invocation to the collector. The options in the invocation are reset, // which ensures that the compiler won't create new dependency collectors, // and thus won't write out the extra '.d' files to disk. auto Opts = std::make_unique(); + + // We need at least one -MT equivalent for the generator of make dependency + // files to work. std::swap(*Opts, ScanInstance.getInvocation().getDependencyOutputOpts()); if (Opts->Targets.empty()) Opts->Targets = {deduceDepTarget(ScanInstance.getFrontendOpts().OutputFile, @@ -603,29 +606,24 @@ getDependencyOutputOptions(CompilerInstance &ScanInstance) { std::shared_ptr initializeScanInstanceDependencyCollector( CompilerInstance &ScanInstance, - const DependencyOutputOptions &DepOutputOpts, StringRef WorkingDirectory, - DependencyConsumer &Consumer, DependencyScanningService &Service, - CompilerInvocation &Inv, DependencyActionController &Controller, + std::unique_ptr DepOutputOpts, + StringRef WorkingDirectory, DependencyConsumer &Consumer, + DependencyScanningService &Service, CompilerInvocation &Inv, + DependencyActionController &Controller, PrebuiltModulesAttrsMap PrebuiltModulesASTMap, llvm::SmallVector &StableDirs) { - // Create the dependency collector that will collect the produced - // dependencies. May return the created ModuleDepCollector depending - // on the scanning format. - - auto Opts = std::make_unique(DepOutputOpts); - std::shared_ptr MDC; switch (Service.getFormat()) { case ScanningOutputFormat::Make: ScanInstance.addDependencyCollector( std::make_shared( - std::move(Opts), WorkingDirectory, Consumer)); + std::move(DepOutputOpts), WorkingDirectory, Consumer)); break; case ScanningOutputFormat::P1689: case ScanningOutputFormat::Full: MDC = std::make_shared( - Service, std::move(Opts), ScanInstance, Consumer, Controller, Inv, - std::move(PrebuiltModulesASTMap), StableDirs); + Service, std::move(DepOutputOpts), ScanInstance, Consumer, Controller, + Inv, std::move(PrebuiltModulesASTMap), StableDirs); ScanInstance.addDependencyCollector(MDC); break; } @@ -676,11 +674,12 @@ bool DependencyScanningAction::runInvocation( if (!MaybePrebuiltModulesASTMap) return false; - auto DepOutputOpts = getDependencyOutputOptions(ScanInstance); + auto DepOutputOpts = takeDependencyOutputOptionsFrom(ScanInstance); MDC = initializeScanInstanceDependencyCollector( - ScanInstance, *DepOutputOpts, WorkingDirectory, Consumer, Service, - OriginalInvocation, Controller, *MaybePrebuiltModulesASTMap, StableDirs); + ScanInstance, std::move(DepOutputOpts), WorkingDirectory, Consumer, + Service, OriginalInvocation, Controller, *MaybePrebuiltModulesASTMap, + StableDirs); std::unique_ptr Action; diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h index 9b79aa11f08b6..71c6731803597 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h @@ -136,13 +136,17 @@ computePrebuiltModulesASTMap(CompilerInstance &ScanInstance, SmallVector &StableDirs); std::unique_ptr -getDependencyOutputOptions(CompilerInstance &ScanInstance); +takeDependencyOutputOptionsFrom(CompilerInstance &ScanInstance); +/// Create the dependency collector that will collect the produced +/// dependencies. May return the created ModuleDepCollector depending +/// on the scanning format. std::shared_ptr initializeScanInstanceDependencyCollector( CompilerInstance &ScanInstance, - const DependencyOutputOptions &DepOutputOpts, StringRef WorkingDirectory, - DependencyConsumer &Consumer, DependencyScanningService &Service, - CompilerInvocation &Inv, DependencyActionController &Controller, + std::unique_ptr DepOutputOpts, + StringRef WorkingDirectory, DependencyConsumer &Consumer, + DependencyScanningService &Service, CompilerInvocation &Inv, + DependencyActionController &Controller, PrebuiltModulesAttrsMap PrebuiltModulesASTMap, llvm::SmallVector &StableDirs); } // namespace dependencies From 36d46769fff8a6715a072833e143d76d128a8a5d Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Thu, 2 Oct 2025 14:01:05 -0700 Subject: [PATCH 5/5] Address review comments. --- clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp index 2652edd2c0b51..e6da7f8899a9a 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp @@ -592,10 +592,9 @@ takeDependencyOutputOptionsFrom(CompilerInstance &ScanInstance) { // which ensures that the compiler won't create new dependency collectors, // and thus won't write out the extra '.d' files to disk. auto Opts = std::make_unique(); - + std::swap(*Opts, ScanInstance.getInvocation().getDependencyOutputOpts()); // We need at least one -MT equivalent for the generator of make dependency // files to work. - std::swap(*Opts, ScanInstance.getInvocation().getDependencyOutputOpts()); if (Opts->Targets.empty()) Opts->Targets = {deduceDepTarget(ScanInstance.getFrontendOpts().OutputFile, ScanInstance.getFrontendOpts().Inputs)};