llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Naveen Seth Hanig (naveen-seth) <details> <summary>Changes</summary> This is the final patch in a series that removes the dependency of `clangDependencyScanning` on `clangDriver`, splitting the work from #<!-- -->169964 into smaller changes (see comment linked below). This patch updates the remaining parts of the scanning interface in `DependencyScanningWorker` to only operate on `-cc1` / driver job command lines. This follows #<!-- -->171238, which applied this change to the by-name scanning API. This is part of a broader effort to support driver-managed builds for compilations using C++ named modules and/or Clang modules. It is required for linking the dependency scanning tooling against the driver without introducing cyclic dependencies, which would otherwise cause build failures when dynamic linking is enabled. --- Patch is 32.72 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/172347.diff 7 Files Affected: - (modified) clang/include/clang/DependencyScanning/DependencyScannerImpl.h (-18) - (modified) clang/include/clang/DependencyScanning/DependencyScanningWorker.h (+41-17) - (modified) clang/lib/DependencyScanning/CMakeLists.txt (-1) - (modified) clang/lib/DependencyScanning/DependencyScannerImpl.cpp (-91) - (modified) clang/lib/DependencyScanning/DependencyScanningWorker.cpp (+94-87) - (modified) clang/lib/Tooling/DependencyScanningTool.cpp (+129-24) - (modified) clang/unittests/DependencyScanning/DependencyScanningWorkerTest.cpp (+7-5) ``````````diff diff --git a/clang/include/clang/DependencyScanning/DependencyScannerImpl.h b/clang/include/clang/DependencyScanning/DependencyScannerImpl.h index f43c7f70183fd..231873375a264 100644 --- a/clang/include/clang/DependencyScanning/DependencyScannerImpl.h +++ b/clang/include/clang/DependencyScanning/DependencyScannerImpl.h @@ -89,28 +89,10 @@ struct TextDiagnosticsPrinterWithOutput { DiagPrinter(DiagnosticsOS, *DiagOpts) {} }; -std::pair<std::unique_ptr<driver::Driver>, std::unique_ptr<driver::Compilation>> -buildCompilation(ArrayRef<std::string> ArgStrs, DiagnosticsEngine &Diags, - IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, - llvm::BumpPtrAllocator &Alloc); - std::unique_ptr<CompilerInvocation> createCompilerInvocation(ArrayRef<std::string> CommandLine, DiagnosticsEngine &Diags); -std::pair<IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem>, - std::vector<std::string>> -initVFSForTUBufferScanning(IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, - ArrayRef<std::string> CommandLine, - StringRef WorkingDirectory, - llvm::MemoryBufferRef TUBuffer); - -std::pair<IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem>, - std::vector<std::string>> -initVFSForByNameScanning(IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, - ArrayRef<std::string> CommandLine, - StringRef WorkingDirectory, StringRef ModuleName); - bool initializeScanCompilerInstance( CompilerInstance &ScanInstance, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, diff --git a/clang/include/clang/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/DependencyScanning/DependencyScanningWorker.h index 489fba4ed3f6b..35f289cf12eb0 100644 --- a/clang/include/clang/DependencyScanning/DependencyScanningWorker.h +++ b/clang/include/clang/DependencyScanning/DependencyScanningWorker.h @@ -16,7 +16,7 @@ #include "clang/DependencyScanning/DependencyScanningService.h" #include "clang/DependencyScanning/ModuleDepCollector.h" #include "clang/Frontend/PCHContainerOperations.h" -#include "llvm/Support/Error.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/MemoryBufferRef.h" #include "llvm/Support/VirtualFileSystem.h" @@ -93,11 +93,13 @@ class DependencyScanningWorker { ~DependencyScanningWorker(); - /// Run the dependency scanning tool for a given clang driver command-line, - /// and report the discovered dependencies to the provided consumer. If - /// TUBuffer is not nullopt, it is used as TU input for the dependency - /// scanning. Otherwise, the input should be included as part of the - /// command-line. + /// Run the dependency scanning tool for the given driver job command-line, + /// and report the discovered dependencies to the provided consumer. + /// + /// OverlayFS should be based on the Worker's dependency scanning file-system + /// and can be used to provide any input specified on the command-line as + /// in-memory file. If no overlay file-system is provided, the Worker's + /// dependency scanning file-system is used instead. /// /// \returns false if clang errors occurred (with diagnostics reported to /// \c DiagConsumer), true otherwise. @@ -105,17 +107,25 @@ class DependencyScanningWorker { StringRef WorkingDirectory, ArrayRef<std::string> CommandLine, DependencyConsumer &DepConsumer, DependencyActionController &Controller, DiagnosticConsumer &DiagConsumer, - std::optional<llvm::MemoryBufferRef> TUBuffer = std::nullopt); + llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS = + nullptr); - /// Run the dependency scanning tool for a given clang driver command-line - /// for a specific translation unit via file system or memory buffer. + /// Run the dependency scanning tool for all given driver job command-lines, + /// and report the discovered dependencies to the provided consumer. /// - /// \returns A \c StringError with the diagnostic output if clang errors - /// occurred, success otherwise. - llvm::Error computeDependencies( - StringRef WorkingDirectory, ArrayRef<std::string> CommandLine, - DependencyConsumer &Consumer, DependencyActionController &Controller, - std::optional<llvm::MemoryBufferRef> TUBuffer = std::nullopt); + /// OverlayFS should be based on the Worker's dependency scanning file-system + /// and can be used to provide any input specified on the command-line as + /// in-memory file. If no overlay file-system is provided, the Worker's + /// dependency scanning file-system is used instead. + /// + /// \returns false if clang errors occurred (with diagnostics reported to + /// \c Diags), true otherwise. + bool computeDependencies( + StringRef WorkingDirectory, ArrayRef<ArrayRef<std::string>> CommandLines, + DependencyConsumer &DepConsumer, DependencyActionController &Controller, + DiagnosticsEngine &Diags, + llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS = + nullptr); /// The three method below implements a new interface for by name /// dependency scanning. They together enable the dependency scanning worker @@ -176,12 +186,26 @@ class DependencyScanningWorker { /// Actually carries out the scan. If \c OverlayFS is provided, it must be /// based on top of DepFS. bool scanDependencies( - StringRef WorkingDirectory, ArrayRef<std::string> CommandLine, + StringRef WorkingDirectory, + ArrayRef<ArrayRef<std::string>> CC1CommandLines, DependencyConsumer &Consumer, DependencyActionController &Controller, - DiagnosticConsumer &DC, + DiagnosticsEngine &Diags, IntrusiveRefCntPtr<llvm::vfs::FileSystem> OverlayFS = nullptr); }; +std::pair<IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem>, + std::vector<std::string>> +initVFSForTUBufferScanning(IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, + ArrayRef<std::string> CommandLine, + StringRef WorkingDirectory, + llvm::MemoryBufferRef TUBuffer); + +std::pair<IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem>, + std::vector<std::string>> +initVFSForByNameScanning(IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, + ArrayRef<std::string> CommandLine, + StringRef WorkingDirectory, StringRef ModuleName); + } // end namespace dependencies } // end namespace clang diff --git a/clang/lib/DependencyScanning/CMakeLists.txt b/clang/lib/DependencyScanning/CMakeLists.txt index 2976f7c236f2e..4c30c3ee57cfd 100644 --- a/clang/lib/DependencyScanning/CMakeLists.txt +++ b/clang/lib/DependencyScanning/CMakeLists.txt @@ -20,7 +20,6 @@ add_clang_library(clangDependencyScanning LINK_LIBS clangAST clangBasic - clangDriver clangFrontend clangLex clangSerialization diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp index 201230d7d6a8e..7fb214eb2e630 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -379,42 +379,6 @@ DiagnosticsEngineWithDiagOpts::DiagnosticsEngineWithDiagOpts( /*ShouldOwnClient=*/false); } -std::pair<std::unique_ptr<driver::Driver>, std::unique_ptr<driver::Compilation>> -dependencies::buildCompilation(ArrayRef<std::string> ArgStrs, - DiagnosticsEngine &Diags, - IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, - llvm::BumpPtrAllocator &Alloc) { - SmallVector<const char *, 256> Argv; - Argv.reserve(ArgStrs.size()); - for (const std::string &Arg : ArgStrs) - Argv.push_back(Arg.c_str()); - - std::unique_ptr<driver::Driver> Driver = std::make_unique<driver::Driver>( - Argv[0], llvm::sys::getDefaultTargetTriple(), Diags, - "clang LLVM compiler", FS); - Driver->setTitle("clang_based_tool"); - - 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); - } - - std::unique_ptr<driver::Compilation> Compilation( - Driver->BuildCompilation(Argv)); - if (!Compilation) - return std::make_pair(nullptr, nullptr); - - if (Compilation->containsError()) - return std::make_pair(nullptr, nullptr); - - return std::make_pair(std::move(Driver), std::move(Compilation)); -} - std::unique_ptr<CompilerInvocation> dependencies::createCompilerInvocation(ArrayRef<std::string> CommandLine, DiagnosticsEngine &Diags) { @@ -430,61 +394,6 @@ dependencies::createCompilerInvocation(ArrayRef<std::string> CommandLine, return Invocation; } -std::pair<IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem>, - std::vector<std::string>> -dependencies::initVFSForTUBufferScanning( - IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, - ArrayRef<std::string> CommandLine, StringRef WorkingDirectory, - llvm::MemoryBufferRef TUBuffer) { - // Reset what might have been modified in the previous worker invocation. - BaseFS->setCurrentWorkingDirectory(WorkingDirectory); - - auto OverlayFS = - llvm::makeIntrusiveRefCnt<llvm::vfs::OverlayFileSystem>(BaseFS); - auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); - InMemoryFS->setCurrentWorkingDirectory(WorkingDirectory); - auto InputPath = TUBuffer.getBufferIdentifier(); - InMemoryFS->addFile( - InputPath, 0, llvm::MemoryBuffer::getMemBufferCopy(TUBuffer.getBuffer())); - IntrusiveRefCntPtr<llvm::vfs::FileSystem> InMemoryOverlay = InMemoryFS; - - OverlayFS->pushOverlay(InMemoryOverlay); - std::vector<std::string> ModifiedCommandLine(CommandLine); - ModifiedCommandLine.emplace_back(InputPath); - - return std::make_pair(OverlayFS, ModifiedCommandLine); -} - -std::pair<IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem>, - std::vector<std::string>> -dependencies::initVFSForByNameScanning( - IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, - ArrayRef<std::string> CommandLine, StringRef WorkingDirectory, - 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<llvm::vfs::OverlayFileSystem>(BaseFS); - auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); - 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("")); - IntrusiveRefCntPtr<llvm::vfs::FileSystem> InMemoryOverlay = InMemoryFS; - OverlayFS->pushOverlay(InMemoryOverlay); - - std::vector<std::string> ModifiedCommandLine(CommandLine); - ModifiedCommandLine.emplace_back(FakeInputPath); - - return std::make_pair(OverlayFS, ModifiedCommandLine); -} - bool dependencies::initializeScanCompilerInstance( CompilerInstance &ScanInstance, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, diff --git a/clang/lib/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/DependencyScanning/DependencyScanningWorker.cpp index ef16b14e7cc6e..1aa77c69a9bb5 100644 --- a/clang/lib/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/DependencyScanning/DependencyScanningWorker.cpp @@ -13,6 +13,7 @@ #include "clang/Driver/Driver.h" #include "clang/Driver/Tool.h" #include "clang/Serialization/ObjectFilePCHContainerReader.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/Support/VirtualFileSystem.h" using namespace clang; @@ -40,39 +41,6 @@ DependencyScanningWorker::DependencyScanningWorker( DependencyScanningWorker::~DependencyScanningWorker() = default; DependencyActionController::~DependencyActionController() = default; -llvm::Error DependencyScanningWorker::computeDependencies( - StringRef WorkingDirectory, ArrayRef<std::string> CommandLine, - DependencyConsumer &Consumer, DependencyActionController &Controller, - std::optional<llvm::MemoryBufferRef> TUBuffer) { - // Capture the emitted diagnostics and report them to the client - // in the case of a failure. - TextDiagnosticsPrinterWithOutput DiagPrinterWithOS(CommandLine); - - if (computeDependencies(WorkingDirectory, CommandLine, Consumer, Controller, - DiagPrinterWithOS.DiagPrinter, TUBuffer)) - return llvm::Error::success(); - return llvm::make_error<llvm::StringError>( - DiagPrinterWithOS.DiagnosticsOS.str(), llvm::inconvertibleErrorCode()); -} - -static bool forEachDriverJob( - ArrayRef<std::string> ArgStrs, DiagnosticsEngine &Diags, - IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, - llvm::function_ref<bool(const driver::Command &Cmd)> Callback) { - // Compilation holds a non-owning a reference to the Driver, hence we need to - // keep the Driver alive when we use Compilation. Arguments to commands may be - // owned by Alloc when expanded from response files. - llvm::BumpPtrAllocator Alloc; - auto [Driver, Compilation] = buildCompilation(ArgStrs, Diags, FS, Alloc); - if (!Compilation) - return false; - for (const driver::Command &Job : Compilation->getJobs()) { - if (!Callback(Job)) - return false; - } - return true; -} - static bool createAndRunToolInvocation( ArrayRef<std::string> CommandLine, DependencyScanningAction &Action, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, @@ -88,11 +56,11 @@ static bool createAndRunToolInvocation( } bool DependencyScanningWorker::scanDependencies( - StringRef WorkingDirectory, ArrayRef<std::string> CommandLine, + StringRef WorkingDirectory, ArrayRef<ArrayRef<std::string>> CC1CommandLines, DependencyConsumer &Consumer, DependencyActionController &Controller, - DiagnosticConsumer &DC, - IntrusiveRefCntPtr<llvm::vfs::FileSystem> OverlayFS) { - IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = DepFS; + DiagnosticsEngine &Diags, + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> OverlayFS) { + IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = nullptr; if (OverlayFS) { #ifndef NDEBUG bool SawDepFS = false; @@ -101,71 +69,55 @@ bool DependencyScanningWorker::scanDependencies( assert(SawDepFS && "OverlayFS not based on DepFS"); #endif FS = std::move(OverlayFS); + } else { + FS = DepFS; + FS->setCurrentWorkingDirectory(WorkingDirectory); } - DiagnosticsEngineWithDiagOpts DiagEngineWithCmdAndOpts(CommandLine, FS, DC); DependencyScanningAction Action(Service, WorkingDirectory, Consumer, Controller, DepFS); - bool Success = false; - if (CommandLine[1] == "-cc1") { - Success = - createAndRunToolInvocation(CommandLine, Action, FS, PCHContainerOps, - *DiagEngineWithCmdAndOpts.DiagEngine); - } else { - Success = forEachDriverJob( - CommandLine, *DiagEngineWithCmdAndOpts.DiagEngine, FS, - [&](const driver::Command &Cmd) { - if (StringRef(Cmd.getCreator().getName()) != "clang") { - // Non-clang command. Just pass through to the dependency - // consumer. - Consumer.handleBuildCommand( - {Cmd.getExecutable(), - {Cmd.getArguments().begin(), Cmd.getArguments().end()}}); - return true; - } - - // Insert -cc1 command line options into Argv - std::vector<std::string> Argv; - Argv.push_back(Cmd.getExecutable()); - llvm::append_range(Argv, Cmd.getArguments()); - - // Create an invocation that uses the underlying file - // 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, - *DiagEngineWithCmdAndOpts.DiagEngine); - }); - } - - if (Success && !Action.hasScanned()) - DiagEngineWithCmdAndOpts.DiagEngine->Report( - diag::err_fe_expected_compiler_job) - << llvm::join(CommandLine, " "); + const bool Success = llvm::all_of(CC1CommandLines, [&](const auto &Cmd) { + if (StringRef(Cmd[1]) != "-cc1") { + // Non-clang command. Just pass through to the dependency consumer. + Consumer.handleBuildCommand({Cmd.front(), {Cmd.begin() + 1, Cmd.end()}}); + return true; + } + // Create an invocation that uses the underlying file system to ensure that + // any file system requests that are made by the driver do not go through + // the dependency scanning filesystem. + return createAndRunToolInvocation(Cmd, Action, FS, PCHContainerOps, Diags); + }); // Ensure finish() is called even if we never reached ExecuteAction(). if (!Action.hasDiagConsumerFinished()) - DC.finish(); + Diags.getClient()->finish(); return Success && Action.hasScanned(); } bool DependencyScanningWorker::computeDependencies( StringRef WorkingDirectory, ArrayRef<std::string> CommandLine, - DependencyConsumer &Consumer, DependencyActionController &Controller, - DiagnosticConsumer &DC, std::optional<llvm::MemoryBufferRef> TUBuffer) { - if (TUBuffer) { - auto [FinalFS, FinalCommandLine] = initVFSForTUBufferScanning( - DepFS, CommandLine, WorkingDirectory, *TUBuffer); - return scanDependencies(WorkingDirectory, FinalCommandLine, Consumer, - Controller, DC, FinalFS); - } + DependencyConsumer &DepConsumer, DependencyActionController &Controller, + DiagnosticConsumer &DiagConsumer, + llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS) { + DiagnosticsEngineWithDiagOpts DiagEngineWithDiagOpts = + OverlayFS + ? DiagnosticsEngineWithDiagOpts(CommandLine, OverlayFS, DiagConsumer) + : DiagnosticsEngineWithDiagOpts(CommandLine, DepFS, DiagConsumer); + auto &Diags = *DiagEngineWithDiagOpts.DiagEngine; + return computeDependencies(WorkingDirectory, + ArrayRef<ArrayRef<std::string>>(CommandLine), + DepConsumer, Controller, Diags, OverlayFS); +} - DepFS->setCurrentWorkingDirectory(WorkingDirectory); - return scanDependencies(WorkingDirectory, CommandLine, Consumer, Controller, - DC); +bool DependencyScanningWorker::computeDependencies( + StringRef WorkingDirectory, ArrayRef<ArrayRef<std::string>> CommandLines, + DependencyConsumer &DepConsumer, DependencyActionController &Controller, + DiagnosticsEngine &Diags, + llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS) { + return scanDependencies(WorkingDirectory, CommandLines, DepConsumer, + Controller, Diags, OverlayFS); } bool DependencyScanningWorker::initializeCompilerInstanceWithContext( @@ -203,3 +155,58 @@ bool DependencyScanningWorker::computeDependenciesByNameWithContext( bool DependencyScanningWorker::finalizeCompilerInstanceWithContext() { return CIWithContext->finalize(); } + +std::pair<IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem>, + std::vector<std::string>> +dependencies::initVFSForTUBufferScanning( + IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, + ArrayRef<std::string> CommandLine, StringRef WorkingDirectory, + llvm::MemoryBufferRef TUBuffer) { + // Reset what might have been modified in the previous worker invocation. + BaseFS->setCurrentWorkingDirectory(WorkingDirectory); + + auto OverlayFS = + l... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/172347 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
