-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[clang][DependencyScanning] Remove dependency on clangDriver from clangDependencyScanning #169964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[clang][DependencyScanning] Remove dependency on clangDriver from clangDependencyScanning #169964
Conversation
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-tools-extra Author: Naveen Seth Hanig (naveen-seth) ChangesThis follows PR #169962 and removes the dependency on
Because 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. The RFC for this change can be found here: Patch is 119.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/169964.diff 32 Files Affected:
diff --git a/clang-tools-extra/clangd/ScanningProjectModules.cpp b/clang-tools-extra/clangd/ScanningProjectModules.cpp
index 672e99632019d..6a21ad2920764 100644
--- a/clang-tools-extra/clangd/ScanningProjectModules.cpp
+++ b/clang-tools-extra/clangd/ScanningProjectModules.cpp
@@ -8,8 +8,8 @@
#include "ProjectModules.h"
#include "support/Logger.h"
-#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
-#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+#include "clang/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanningTool.h"
namespace clang::clangd {
namespace {
@@ -36,8 +36,8 @@ class ModuleDependencyScanner {
std::shared_ptr<const clang::tooling::CompilationDatabase> CDB,
const ThreadsafeFS &TFS)
: CDB(CDB), TFS(TFS),
- Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
- tooling::dependencies::ScanningOutputFormat::P1689) {}
+ Service(dependencies::ScanningMode::CanonicalPreprocessing,
+ dependencies::ScanningOutputFormat::P1689) {}
/// The scanned modules dependency information for a specific source file.
struct ModuleDependencyInfo {
@@ -81,7 +81,7 @@ class ModuleDependencyScanner {
// Whether the scanner has scanned the project globally.
bool GlobalScanned = false;
- clang::tooling::dependencies::DependencyScanningService Service;
+ clang::dependencies::DependencyScanningService Service;
// TODO: Add a scanning cache.
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h b/clang/include/clang/DependencyScanning/DependencyScannerImpl.h
similarity index 76%
rename from clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h
rename to clang/include/clang/DependencyScanning/DependencyScannerImpl.h
index b94d1b472f920..61d0aa59a3fb1 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h
+++ b/clang/include/clang/DependencyScanning/DependencyScannerImpl.h
@@ -9,18 +9,17 @@
#ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNER_H
#define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNER_H
+#include "clang/DependencyScanning/DependencyScanningFilesystem.h"
+#include "clang/DependencyScanning/ModuleDepCollector.h"
#include "clang/Driver/Compilation.h"
+#include "clang/Driver/Driver.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"
namespace clang {
class DiagnosticConsumer;
-namespace tooling {
namespace dependencies {
class DependencyScanningService;
class DependencyScanningWorker;
@@ -103,27 +102,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::FileSystem>, 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,
@@ -158,22 +140,11 @@ class CompilerInstanceWithContext {
llvm::StringRef CWD;
std::vector<std::string> CommandLine;
- // Context - file systems
- llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS;
-
// Context - Diagnostics engine.
- std::unique_ptr<TextDiagnosticsPrinterWithOutput> DiagPrinterWithOS;
- // DiagConsumer may points to DiagPrinterWithOS->DiagPrinter, or a custom
- // DiagnosticConsumer passed in from initialize.
DiagnosticConsumer *DiagConsumer = nullptr;
std::unique_ptr<DignosticsEngineWithDiagOpts> DiagEngineWithCmdAndOpts;
// Context - compiler invocation
- // Compilation's command's arguments may be owned by Alloc when expanded from
- // response files, so we need to keep Alloc alive in the context.
- llvm::BumpPtrAllocator Alloc;
- std::unique_ptr<clang::driver::Driver> Driver;
- std::unique_ptr<clang::driver::Compilation> Compilation;
std::unique_ptr<CompilerInvocation> OriginalInvocation;
// Context - output options
@@ -195,18 +166,15 @@ class CompilerInstanceWithContext {
: Worker(Worker), CWD(CWD), CommandLine(CMD) {};
// The three methods below returns false when they fail, with the detail
- // accumulated in DiagConsumer.
- bool initialize(DiagnosticConsumer *DC);
+ // accumulated in \c DiagEngineWithDiagOpts's diagnostic consumer.
+ bool initialize(
+ std::unique_ptr<DignosticsEngineWithDiagOpts> DiagEngineWithDiagOpts,
+ IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
bool computeDependencies(StringRef ModuleName, DependencyConsumer &Consumer,
DependencyActionController &Controller);
bool finalize();
-
- // The method below turns the return status from the above methods
- // into an llvm::Error using a default DiagnosticConsumer.
- llvm::Error handleReturnStatus(bool Success);
};
} // namespace dependencies
-} // namespace tooling
} // namespace clang
#endif
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/DependencyScanning/DependencyScanningFilesystem.h
similarity index 98%
rename from clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
rename to clang/include/clang/DependencyScanning/DependencyScanningFilesystem.h
index 2b21be7712693..a4516ff77509d 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ b/clang/include/clang/DependencyScanning/DependencyScanningFilesystem.h
@@ -1,4 +1,4 @@
-//===- DependencyScanningFilesystem.h - clang-scan-deps fs ===---*- C++ -*-===//
+//===- DependencyScanningFilesystem.h - Optimized Scanning FS ---*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -6,8 +6,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGFILESYSTEM_H
-#define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGFILESYSTEM_H
+#ifndef LLVM_CLANG_DEPENDENCYSCANNING_DEPENDENCYSCANNINGFILESYSTEM_H
+#define LLVM_CLANG_DEPENDENCYSCANNING_DEPENDENCYSCANNINGFILESYSTEM_H
#include "clang/Basic/LLVM.h"
#include "clang/Lex/DependencyDirectivesScanner.h"
@@ -21,7 +21,6 @@
#include <variant>
namespace clang {
-namespace tooling {
namespace dependencies {
using DependencyDirectivesTy =
@@ -521,7 +520,6 @@ class DependencyScanningWorkerFilesystem
};
} // end namespace dependencies
-} // end namespace tooling
} // end namespace clang
-#endif // LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGFILESYSTEM_H
+#endif // LLVM_CLANG_DEPENDENCYSCANNING_DEPENDENCYSCANNINGFILESYSTEM_H
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/DependencyScanning/DependencyScanningService.h
similarity index 89%
rename from clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
rename to clang/include/clang/DependencyScanning/DependencyScanningService.h
index 4e97c7bc9f36e..96dd33c28cf5a 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/DependencyScanning/DependencyScanningService.h
@@ -1,4 +1,4 @@
-//===- DependencyScanningService.h - clang-scan-deps service ===-*- C++ -*-===//
+//===- DependencyScanningService.h - Scanning Service -----------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -6,16 +6,15 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
-#define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
+#ifndef LLVM_CLANG_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
+#define LLVM_CLANG_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
-#include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
-#include "clang/Tooling/DependencyScanning/InProcessModuleCache.h"
+#include "clang/DependencyScanning/DependencyScanningFilesystem.h"
+#include "clang/DependencyScanning/InProcessModuleCache.h"
#include "llvm/ADT/BitmaskEnum.h"
#include "llvm/Support/Chrono.h"
namespace clang {
-namespace tooling {
namespace dependencies {
/// The mode in which the dependency scanner will operate to find the
@@ -125,7 +124,6 @@ class DependencyScanningService {
};
} // end namespace dependencies
-} // end namespace tooling
} // end namespace clang
-#endif // LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
+#endif // LLVM_CLANG_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
diff --git a/clang/include/clang/DependencyScanning/DependencyScanningUtils.h b/clang/include/clang/DependencyScanning/DependencyScanningUtils.h
new file mode 100644
index 0000000000000..e2fb5ad3e5cf3
--- /dev/null
+++ b/clang/include/clang/DependencyScanning/DependencyScanningUtils.h
@@ -0,0 +1,166 @@
+//===- DependencyScanningUtils.h - Common Scanning Utilities ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_DEPENDENCYSCANNING_DEPENDENCYSCANNINGUTILS_H
+#define LLVM_CLANG_DEPENDENCYSCANNING_DEPENDENCYSCANNINGUTILS_H
+
+#include "clang/DependencyScanning/DependencyScannerImpl.h"
+#include "clang/DependencyScanning/DependencyScanningWorker.h"
+#include "clang/DependencyScanning/ModuleDepCollector.h"
+
+namespace clang {
+namespace dependencies {
+
+/// Graph of modular dependencies.
+using ModuleDepsGraph = std::vector<clang::dependencies::ModuleDeps>;
+
+/// The full dependencies and module graph for a specific input.
+struct TranslationUnitDeps {
+ /// The graph of direct and transitive modular dependencies.
+ ModuleDepsGraph ModuleGraph;
+
+ /// The identifier of the C++20 module this translation unit exports.
+ ///
+ /// If the translation unit is not a module then \c ID.ModuleName is empty.
+ clang::dependencies::ModuleID ID;
+
+ /// A collection of absolute paths to files that this translation unit
+ /// directly depends on, not including transitive dependencies.
+ std::vector<std::string> FileDeps;
+
+ /// A collection of prebuilt modules this translation unit directly depends
+ /// on, not including transitive dependencies.
+ std::vector<clang::dependencies::PrebuiltModuleDep> PrebuiltModuleDeps;
+
+ /// A list of modules this translation unit directly depends on, not including
+ /// transitive dependencies.
+ ///
+ /// This may include modules with a different context hash when it can be
+ /// determined that the differences are benign for this compilation.
+ std::vector<clang::dependencies::ModuleID> ClangModuleDeps;
+
+ /// A list of module names that are visible to this translation unit. This
+ /// includes both direct and transitive module dependencies.
+ std::vector<std::string> VisibleModules;
+
+ /// A list of the C++20 named modules this translation unit depends on.
+ std::vector<std::string> NamedModuleDeps;
+
+ /// The sequence of commands required to build the translation unit. Commands
+ /// should be executed in order.
+ ///
+ /// FIXME: If we add support for multi-arch builds in clang-scan-deps, we
+ /// should make the dependencies between commands explicit to enable parallel
+ /// builds of each architecture.
+ std::vector<clang::dependencies::Command> Commands;
+
+ /// Deprecated driver command-line. This will be removed in a future version.
+ std::vector<std::string> DriverCommandLine;
+};
+
+class FullDependencyConsumer : public clang::dependencies::DependencyConsumer {
+public:
+ FullDependencyConsumer(
+ const llvm::DenseSet<clang::dependencies::ModuleID> &AlreadySeen)
+ : AlreadySeen(AlreadySeen) {}
+
+ void handleBuildCommand(clang::dependencies::Command Cmd) override {
+ Commands.push_back(std::move(Cmd));
+ }
+
+ void handleDependencyOutputOpts(const DependencyOutputOptions &) override {}
+
+ void handleFileDependency(StringRef File) override {
+ Dependencies.push_back(std::string(File));
+ }
+
+ void handlePrebuiltModuleDependency(
+ clang::dependencies::PrebuiltModuleDep PMD) override {
+ PrebuiltModuleDeps.emplace_back(std::move(PMD));
+ }
+
+ void handleModuleDependency(clang::dependencies::ModuleDeps MD) override {
+ ClangModuleDeps[MD.ID] = std::move(MD);
+ }
+
+ void handleDirectModuleDependency(clang::dependencies::ModuleID ID) override {
+ DirectModuleDeps.push_back(ID);
+ }
+
+ void handleVisibleModule(std::string ModuleName) override {
+ VisibleModules.push_back(ModuleName);
+ }
+
+ void handleContextHash(std::string Hash) override {
+ ContextHash = std::move(Hash);
+ }
+
+ void handleProvidedAndRequiredStdCXXModules(
+ std::optional<clang::dependencies::P1689ModuleInfo> Provided,
+ std::vector<clang::dependencies::P1689ModuleInfo> Requires) override {
+ ModuleName = Provided ? Provided->ModuleName : "";
+ llvm::transform(Requires, std::back_inserter(NamedModuleDeps),
+ [](const auto &Module) { return Module.ModuleName; });
+ }
+
+ TranslationUnitDeps takeTranslationUnitDeps();
+
+private:
+ std::vector<std::string> Dependencies;
+ std::vector<clang::dependencies::PrebuiltModuleDep> PrebuiltModuleDeps;
+ llvm::MapVector<clang::dependencies::ModuleID,
+ clang::dependencies::ModuleDeps>
+ ClangModuleDeps;
+ std::string ModuleName;
+ std::vector<std::string> NamedModuleDeps;
+ std::vector<clang::dependencies::ModuleID> DirectModuleDeps;
+ std::vector<std::string> VisibleModules;
+ std::vector<clang::dependencies::Command> Commands;
+ std::string ContextHash;
+ const llvm::DenseSet<clang::dependencies::ModuleID> &AlreadySeen;
+};
+
+/// A callback to lookup module outputs for "-fmodule-file=", "-o" etc.
+using LookupModuleOutputCallback =
+ llvm::function_ref<std::string(const clang::dependencies::ModuleDeps &,
+ clang::dependencies::ModuleOutputKind)>;
+
+/// A simple dependency action controller that uses a callback. If no callback
+/// is provided, it is assumed that looking up module outputs is unreachable.
+class CallbackActionController
+ : public clang::dependencies::DependencyActionController {
+public:
+ virtual ~CallbackActionController();
+
+ static std::string
+ lookupUnreachableModuleOutput(const clang::dependencies::ModuleDeps &MD,
+ clang::dependencies::ModuleOutputKind Kind) {
+ llvm::report_fatal_error("unexpected call to lookupModuleOutput");
+ };
+
+ CallbackActionController(LookupModuleOutputCallback LMO)
+ : LookupModuleOutput(std::move(LMO)) {
+ if (!LookupModuleOutput) {
+ LookupModuleOutput = lookupUnreachableModuleOutput;
+ }
+ }
+
+ std::string
+ lookupModuleOutput(const clang::dependencies::ModuleDeps &MD,
+ clang::dependencies::ModuleOutputKind Kind) override {
+ return LookupModuleOutput(MD, Kind);
+ }
+
+private:
+ LookupModuleOutputCallback LookupModuleOutput;
+};
+
+} // end namespace dependencies
+} // end namespace clang
+
+#endif // LLVM_CLANG_DEPENDENCYSCANNING_DEPENDENCYSCANNINGUTILS_H
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/DependencyScanning/DependencyScanningWorker.h
similarity index 69%
rename from clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
rename to clang/include/clang/DependencyScanning/DependencyScanningWorker.h
index e2c353a254bf3..0f1b78648342f 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/DependencyScanning/DependencyScanningWorker.h
@@ -1,4 +1,4 @@
-//===- DependencyScanningWorker.h - clang-scan-deps worker ===---*- C++ -*-===//
+//===- DependencyScanningWorker.h - Thread-Safe Scanning Worker -*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -6,15 +6,17 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGWORKER_H
-#define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGWORKER_H
+#ifndef LLVM_CLANG_DEPENDENCYSCANNING_DEPENDENCYSCANNINGWORKER_H
+#define LLVM_CLANG_DEPENDENCYSCANNING_DEPENDENCYSCANNINGWORKER_H
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/LLVM.h"
+#include "clang/DependencyScanning/DependencyScannerImpl.h"
+#include "clang/DependencyScanning/DependencyScanningService.h"
+#include "clang/DependencyScanning/ModuleDepCollector.h"
#include "clang/Frontend/PCHContainerOperations.h"
-#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
-#include "clang/Tooling/DependencyScanning/ModuleDepCollector.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBufferRef.h"
@@ -25,7 +27,6 @@ namespace clang {
class DependencyOutputOptions;
-namespace tooling {
namespace dependencies {
class DependencyScanningWorkerFilesystem;
@@ -92,41 +93,56 @@ 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 a given clang -cc1 command-line,
+ /// and report the discovered dependencies to the provided consumer.
///
- /// \returns false if clang errors occurred (with diagnostics reported to
+ /// @return false if clang errors occurred (with diagnostics reported to
/// \c DiagConsumer), true otherwise.
bool computeDependencies(
StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
DependencyConsumer &DepConsumer, DependencyActionController &Controller,
DiagnosticConsumer &DiagConsumer,
- std::optional<llvm::MemoryBufferRef> TUBuffer = std::nullopt);
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> ScanFS = 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 clang -cc1 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, const std::vector<std::string> &CommandLine,
+ ...
[truncated]
|
…ndencyScanningTool (NFC) (#169962) This patch is the first of two in refactoring Clang's dependency scanning tooling to remove its dependency on clangDriver. It separates Tooling/DependencyScanningTool.cpp from the rest of clangDependencyScanning and moves clangDependencyScanning out of clangTooling into its own library. No functional changes are introduced. The follow-up patch (#169964) will restrict clangDependencyScanning to handling only -cc1 command line inputs and will move all functionality related to handling driver commands into clangTooling. (Tooling/DependencyScanningTool.cpp). 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. The RFC for this change can be found here: https://discourse.llvm.org/t/rfc-new-clangoptions-library-remove-dependency-on-clangdriver-from-clangfrontend-and-flangfrontend/88773?u=naveen-seth
…ngDependencyScanning This follows PR llvm#169962 and removes the dependency on clangDriver from clangDependencyScanning. DependencyScanningWorker now only supports -cc1 command line inputs, and all functionality related to driver-level commands has been moved into clangTooling (DependencyScanningTool.cpp). Because DependencyScanningWorker now only accepts -cc1 inputs, this patch enables the use of -cc1 commands with the by-name scanning API introduced in llvm#164345. 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. The RFC for this change can be found here: https://discourse.llvm.org/t/rfc-new-clangoptions-library-remove-dependency-on-clangdriver-from-clangfrontend-and-flangfrontend/88773?u=naveen-seth
58ff0e5 to
99dd142
Compare
…ndencyScanningTool (NFC) (llvm#169962) This patch is the first of two in refactoring Clang's dependency scanning tooling to remove its dependency on clangDriver. It separates Tooling/DependencyScanningTool.cpp from the rest of clangDependencyScanning and moves clangDependencyScanning out of clangTooling into its own library. No functional changes are introduced. The follow-up patch (llvm#169964) will restrict clangDependencyScanning to handling only -cc1 command line inputs and will move all functionality related to handling driver commands into clangTooling. (Tooling/DependencyScanningTool.cpp). 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. The RFC for this change can be found here: https://discourse.llvm.org/t/rfc-new-clangoptions-library-remove-dependency-on-clangdriver-from-clangfrontend-and-flangfrontend/88773?u=naveen-seth
jansvoboda11
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense conceptually, but I'd like to see more prep commits so that this one is easier to review. I commented on what I think could be extracted out of this PR. I'll defer the full review to @qiongsiwu, who I believe had reasons to put some functions to the header file while you move them to the implementation file (initVFSForTUBufferScanning for example).
| std::unique_ptr<FrontendAction> Action = | ||
| std::make_unique<PreprocessOnlyAction>(); | ||
| auto InputFile = CI.getFrontendOpts().Inputs.begin(); | ||
| auto *InputFile = CI.getFrontendOpts().Inputs.begin(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be better to do in a separate commit.
|
|
||
| bool DependencyScanningWorker::scanDependencies( | ||
| StringRef WorkingDirectory, const std::vector<std::string> &CommandLine, | ||
| StringRef WorkingDirectory, ArrayRef<std::vector<std::string>> CommandLines, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be better to do in a separate commit.
| bool SawDepFS = false; | ||
| FS->visit( | ||
| [&](llvm::vfs::FileSystem &VFS) { SawDepFS |= &VFS == DepFS.get(); }); | ||
| assert(SawDepFS && "FS not based on DepFS"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rename and non-null FS might be better to do in a separate commit.
clang/lib/Tooling/CMakeLists.txt
Outdated
| clangBasic | ||
| clangDependencyScanning | ||
| clangDriver | ||
| clangDependencyScanning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate.
| "file": "", | ||
| "directory": "DIR", | ||
| "command": "clang -fmodules -fmodules-cache-path=DIR/cache -I DIR -x c" | ||
| "command": "clang -fmodules -fmodules-cache-path=DIR/cache -I DIR -x c main.cpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the entire point of the by-module-name scan is that you don't need an input file. Why is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My mistake; I initially thought command-lines with input files were also supported.
I've removed the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on the PR! This overall looks great!
reasons to put some functions to the header file while you move them to the implementation file (
initVFSForTUBufferScanningfor example).
I believe the reason I needed these init function declarations in the header file was that these functions could be called in DependencyScanningWorker.cpp and DependencyScannerImpl.cpp. I can see that we are now moving the initializations to DependencyScanningTool.cpp, so I think it is good to remove these decls from the header file.
I concur with @jansvoboda11 that we can partition this into a few different PRs. I am thinking maybe we can partition the changes into three PRs:
- NFC: cleaning up function parameter types. There are a few places where we are replacing
std::vector &s withArrayRefs. The places Jan pointed out could go into this PR. Other NFC changes can go in here as well. - Refactor the initialization and finalization logic. We can use this PR to move the initialization and finalization code out of the impl/worker files to
DependencyScanningTool.cpp. I suspect it is difficult to make a completely clean cut since things are entangled, but I think we can give this a shot. - Feature: we can now use the new APIs moved to
DependencyScanningTool.cppto removeDependencyScanningWorker's dependency on the driver.
This will not only make the review easier, but then we can merge this into the downstream Swift fork in smaller pieces to reduce merge conflicts.
Does this sound reasonable?
| DependencyConsumer &Consumer, | ||
| DependencyActionController &Controller, | ||
| DiagnosticsEngine &Diags, | ||
| llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: these two function declarations look too similar to my eyes. Maybe we can do something along the following line:
| llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS); | |
| bool scanDependenciesWithDiagConsumer( | |
| DiagnosticConsumer &DC, | |
| StringRef WorkingDirectory, | |
| const std::vector<std::string> &CommandLine, | |
| DependencyConsumer &Consumer, | |
| DependencyActionController &Controller, | |
| llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS); | |
| bool scanDependenciesWithDiagEngine( | |
| DiagnosticsEngine &Diags, | |
| StringRef WorkingDirectory, | |
| ArrayRef<std::vector<std::string>> CommandLine, | |
| DependencyConsumer &Consumer, | |
| DependencyActionController &Controller, | |
| llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS); |
Note the renamed functions, and the reordered parameter lists.
…const std::vector& (NFC) This updates the dependency-scanning tooling to consistently use ArrayRef<std::string> rather than const std::vector<std::string>& in function signatures. This is done to help break PR llvm#169964 into smaller, more manageable pieces.
d2a6306 to
ea5222a
Compare
…nning into DependencyScanningTool This is the second patch in a series that removes the dependency of clangDependencyScanning on clangDriver, splitting the work from llvm#169964 into smaller changes (see comment linked below). This patch updates the by-name scanning interface in DependencyScanningWorker to accept only -cc1 command lines and moves the logic for handling driver-style command lines into DependencyScanningTool in clangTooling. Support for -cc1 command lines in by-name scanning is introduced in this patch. The next patch will update the remaining parts of DependencyScanningWorker to operate only on -cc1 command lines, allowing its dependency on clangDriver to be removed.” llvm#169964 (review)
…nning into DependencyScanningTool This is the second patch in a series that removes the dependency of clangDependencyScanning on clangDriver, splitting the work from llvm#169964 into smaller changes (see comment linked below). This patch updates the by-name scanning interface in DependencyScanningWorker to accept only -cc1 command lines and moves the logic for handling driver-style command lines into DependencyScanningTool in clangTooling. Support for -cc1 command lines in by-name scanning is introduced in this patch. The next patch will update the remaining parts of DependencyScanningWorker to operate only on -cc1 command lines, allowing its dependency on clangDriver to be removed. llvm#169964 (review)
…nning into DependencyScanningTool This is the second patch in a series that removes the dependency of clangDependencyScanning on clangDriver, splitting the work from llvm#169964 into smaller changes (see comment linked below). This patch updates the by-name scanning interface in DependencyScanningWorker to accept only -cc1 command lines and moves the logic for handling driver-style command lines into DependencyScanningTool in clangTooling. Support for -cc1 command lines in by-name scanning is introduced in this patch. The next patch will update the remaining parts of DependencyScanningWorker to operate only on -cc1 command lines, allowing its dependency on clangDriver to be removed. llvm#169964 (review)
…ndencyScanningTool (NFC) (llvm#169962) This patch is the first of two in refactoring Clang's dependency scanning tooling to remove its dependency on clangDriver. It separates Tooling/DependencyScanningTool.cpp from the rest of clangDependencyScanning and moves clangDependencyScanning out of clangTooling into its own library. No functional changes are introduced. The follow-up patch (llvm#169964) will restrict clangDependencyScanning to handling only -cc1 command line inputs and will move all functionality related to handling driver commands into clangTooling. (Tooling/DependencyScanningTool.cpp). 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. The RFC for this change can be found here: https://discourse.llvm.org/t/rfc-new-clangoptions-library-remove-dependency-on-clangdriver-from-clangfrontend-and-flangfrontend/88773?u=naveen-seth
…const std::vector& (NFC) (llvm#170941) This updates the dependency-scanning tooling to consistently use `ArrayRef<std::string>` rather than `const std::vector<std::string>&` in function signatures. This is done to help break PR llvm#169964 into smaller, more manageable pieces.
This follows PR #169962 and removes the dependency on
clangDriverfromclangDependencyScanning.DependencyScanningWorkernow supports only-cc1command line inputs and all functionality for handling driver command line inputs has been moved out ofclangDependencyScanningintoDependencyScanningToolinclangTooling.Because
DependencyScanningWorkernow only supports-cc1inputs, this patch enables the use of-cc1commands with the by-name scanning API introduced in #164345.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.
The RFC for this change can be found here:
https://discourse.llvm.org/t/rfc-new-clangoptions-library-remove-dependency-on-clangdriver-from-clangfrontend-and-flangfrontend/88773?u=naveen-seth