-
Notifications
You must be signed in to change notification settings - Fork 15.7k
[clang][DependencyScanning] Remove dependency on clangDriver from clangDependencyScanning #172347
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
[clang][DependencyScanning] Remove dependency on clangDriver from clangDependencyScanning #172347
Conversation
…ngDependencyScanning This is the final 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 remaining parts of the scanning interface in DependencyScanningWorker to only operate on -cc1 / driver job command lines. This follows llvm#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.
|
@llvm/pr-subscribers-clang Author: Naveen Seth Hanig (naveen-seth) ChangesThis is the final patch in a series that removes the dependency of This patch updates the remaining parts of the scanning interface in 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:
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]
|
clang/include/clang/DependencyScanning/DependencyScanningWorker.h
Outdated
Show resolved
Hide resolved
clang/include/clang/DependencyScanning/DependencyScanningWorker.h
Outdated
Show resolved
Hide resolved
clang/include/clang/DependencyScanning/DependencyScanningWorker.h
Outdated
Show resolved
Hide resolved
Co-authored-by: Jan Svoboda <[email protected]>
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 is great, splitting out a couple of smaller PRs makes this a breeze to review! I only have minor feedback. Let's see what @qiongsiwu has to say too.
clang/include/clang/DependencyScanning/DependencyScanningWorker.h
Outdated
Show resolved
Hide resolved
…rom-dependency-scanning
Now that we are using ArrayRef consistently in the computeDependencies function signatures, we can also use SmallVector over std::vector as per https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h
…ch frontend invocation.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
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.
LGTM with minor suggestions/questions. Great job, thank you!
|
Thanks for reviewing! I’ll merge after @qiongsiwu has had a chance to review this. I just finished resolving the merge conflicts in #152770 locally and noticed that I’ll need to move |
|
I'd just land this PR as-is and deal with the move later. Out of curiosity, WDYT about moving |
|
I’m neutral on this for now, but in general I think this is a good change! In #152770 (comment) and the reply #152770 (comment) there is some discussion about unifying more of the dependency-scanning use-cases. I feel like the driver could be a good place to do this. Doing this change might be unnecessary if it turns out that unifying dependency scanning makes more sense somewhere else, but I can imagine it will take some time to reach a good decision, so I’m not too worried about this conflicting. I am curious to know what others working on dependency scanning think about this, also in regards to the linked discussion. |
…angDependencyScanning (llvm#172347) This is the final 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 remaining parts of the scanning interface in DependencyScanningWorker to only operate on -cc1 / frontend command-lines. This follows llvm#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.
…angDependencyScanning (llvm#172347) This is the final 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 remaining parts of the scanning interface in DependencyScanningWorker to only operate on -cc1 / frontend command-lines. This follows llvm#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.
|
Ping @qiongsiwu |
qiongsiwu
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.
Thanks for the ping! Sorry I missed the notifications on this review.
The changes overall look great! I have one suggestion that facilitates merging this PR to the downstream Swift's llvm fork.
qiongsiwu
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.
LGTM! Thanks!
Thanks for reviewing! |
…ngDependencyScanning (llvm#172347) This is the final 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 remaining parts of the scanning interface in `DependencyScanningWorker` to only operate on `-cc1` / driver job command lines. This follows llvm#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. --------- Co-authored-by: Jan Svoboda <[email protected]>
|
I think this change might be responsible for the failures here: https://lab.llvm.org/buildbot/#/builders/25/builds/14594 I think they are hitting a UBSan check: |
I believe this was fixed by #174515. The corresponding Buildbot build (https://lab.llvm.org/buildbot/#/builders/25/builds/14595) is not hitting the UBSan checks anymore. |
This is the final patch in a series that removes the dependency of
clangDependencyScanningonclangDriver, splitting the work from #169964 into smaller changes (see comment linked below).This patch updates the remaining parts of the scanning interface in
DependencyScanningWorkerto 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.