[clang] [clang][deps] Propagate the entire service (PR #128959)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 27 09:14:04 PST 2025
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/128959
>From cb389c006e5ca28d5baef73625776a3e0e58b8af Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Wed, 26 Feb 2025 15:12:43 -0800
Subject: [PATCH 1/2] [clang][deps] Propagate the whole service
Adding any kind of shared state between dependency scanning workers is done through the service. Until this PR, the first thing each worker does is splitting the service into its members. These are typically threaded through the scanning action and collector, making each change to service members needlessly complicated and spread out. Moreover, downstream forks with extra service members face frequent merge conflicts.
---
.../DependencyScanningWorker.h | 9 +---
.../DependencyScanning/ModuleDepCollector.h | 16 +++----
.../DependencyScanningWorker.cpp | 39 +++++++---------
.../DependencyScanning/ModuleDepCollector.cpp | 45 ++++++++++---------
4 files changed, 45 insertions(+), 64 deletions(-)
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index ee7582b851020..cd9d191e68aeb 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -130,11 +130,11 @@ class DependencyScanningWorker {
DependencyActionController &Controller,
StringRef ModuleName);
- bool shouldEagerLoadModules() const { return EagerLoadModules; }
-
llvm::vfs::FileSystem &getVFS() const { return *BaseFS; }
private:
+ /// The parent dependency scanning service.
+ DependencyScanningService &Service;
std::shared_ptr<PCHContainerOperations> PCHContainerOps;
/// The file system to be used during the scan.
/// This is either \c FS passed in the constructor (when performing canonical
@@ -144,11 +144,6 @@ class DependencyScanningWorker {
/// dependency-directives-extracting) filesystem overlaid on top of \c FS
/// (passed in the constructor).
llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
- ScanningOutputFormat Format;
- /// Whether to optimize the modules' command-line arguments.
- ScanningOptimizations OptimizeArgs;
- /// Whether to set up command-lines to load PCM files eagerly.
- bool EagerLoadModules;
/// Private helper functions.
bool scanDependencies(StringRef WorkingDirectory,
diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index b41efa254342e..20fb4de6a2a73 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -225,13 +225,12 @@ class ModuleDepCollectorPP final : public PPCallbacks {
/// \c ModuleDepCollectorPP to the preprocessor.
class ModuleDepCollector final : public DependencyCollector {
public:
- ModuleDepCollector(std::unique_ptr<DependencyOutputOptions> Opts,
+ ModuleDepCollector(DependencyScanningService &Service,
+ std::unique_ptr<DependencyOutputOptions> Opts,
CompilerInstance &ScanInstance, DependencyConsumer &C,
DependencyActionController &Controller,
CompilerInvocation OriginalCI,
- PrebuiltModuleVFSMapT PrebuiltModuleVFSMap,
- ScanningOptimizations OptimizeArgs, bool EagerLoadModules,
- bool IsStdModuleP1689Format);
+ PrebuiltModuleVFSMapT PrebuiltModuleVFSMap);
void attachToPreprocessor(Preprocessor &PP) override;
void attachToASTReader(ASTReader &R) override;
@@ -243,6 +242,8 @@ class ModuleDepCollector final : public DependencyCollector {
private:
friend ModuleDepCollectorPP;
+ /// The parent dependency scanning service.
+ DependencyScanningService &Service;
/// The compiler instance for scanning the current translation unit.
CompilerInstance &ScanInstance;
/// The consumer of collected dependency information.
@@ -274,13 +275,6 @@ class ModuleDepCollector final : public DependencyCollector {
/// a discovered modular dependency. Note that this still needs to be adjusted
/// for each individual module.
CowCompilerInvocation CommonInvocation;
- /// Whether to optimize the modules' command-line arguments.
- ScanningOptimizations OptimizeArgs;
- /// Whether to set up command-lines to load PCM files eagerly.
- bool EagerLoadModules;
- /// If we're generating dependency output in P1689 format
- /// for standard C++ modules.
- bool IsStdModuleP1689Format;
std::optional<P1689ModuleInfo> ProvidedStdCXXModule;
std::vector<P1689ModuleInfo> RequiredStdCXXModules;
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index ff076a1db0d59..697f26ee5d12f 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -284,15 +284,12 @@ static void canonicalizeDefines(PreprocessorOptions &PPOpts) {
class DependencyScanningAction : public tooling::ToolAction {
public:
DependencyScanningAction(
- StringRef WorkingDirectory, DependencyConsumer &Consumer,
- DependencyActionController &Controller,
+ DependencyScanningService &Service, StringRef WorkingDirectory,
+ DependencyConsumer &Consumer, DependencyActionController &Controller,
llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS,
- ScanningOutputFormat Format, ScanningOptimizations OptimizeArgs,
- bool EagerLoadModules, bool DisableFree,
- std::optional<StringRef> ModuleName = std::nullopt)
- : WorkingDirectory(WorkingDirectory), Consumer(Consumer),
- Controller(Controller), DepFS(std::move(DepFS)), Format(Format),
- OptimizeArgs(OptimizeArgs), EagerLoadModules(EagerLoadModules),
+ bool DisableFree, std::optional<StringRef> ModuleName = std::nullopt)
+ : Service(Service), WorkingDirectory(WorkingDirectory),
+ Consumer(Consumer), Controller(Controller), DepFS(std::move(DepFS)),
DisableFree(DisableFree), ModuleName(ModuleName) {}
bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
@@ -303,7 +300,7 @@ class DependencyScanningAction : public tooling::ToolAction {
CompilerInvocation OriginalInvocation(*Invocation);
// Restore the value of DisableFree, which may be modified by Tooling.
OriginalInvocation.getFrontendOpts().DisableFree = DisableFree;
- if (any(OptimizeArgs & ScanningOptimizations::Macros))
+ if (any(Service.getOptimizeArgs() & ScanningOptimizations::Macros))
canonicalizeDefines(OriginalInvocation.getPreprocessorOpts());
if (Scanned) {
@@ -340,7 +337,7 @@ class DependencyScanningAction : public tooling::ToolAction {
ScanInstance.getFrontendOpts().ModulesShareFileManager = true;
ScanInstance.getHeaderSearchOpts().ModuleFormat = "raw";
ScanInstance.getHeaderSearchOpts().ModulesIncludeVFSUsage =
- any(OptimizeArgs & ScanningOptimizations::VFS);
+ any(Service.getOptimizeArgs() & ScanningOptimizations::VFS);
// Support for virtual file system overlays.
auto FS = createVFSFromCompilerInvocation(
@@ -400,7 +397,7 @@ class DependencyScanningAction : public tooling::ToolAction {
ScanInstance.getFrontendOpts().Inputs)};
Opts->IncludeSystemHeaders = true;
- switch (Format) {
+ switch (Service.getFormat()) {
case ScanningOutputFormat::Make:
ScanInstance.addDependencyCollector(
std::make_shared<DependencyConsumerForwarder>(
@@ -409,9 +406,8 @@ class DependencyScanningAction : public tooling::ToolAction {
case ScanningOutputFormat::P1689:
case ScanningOutputFormat::Full:
MDC = std::make_shared<ModuleDepCollector>(
- std::move(Opts), ScanInstance, Consumer, Controller,
- OriginalInvocation, std::move(PrebuiltModuleVFSMap), OptimizeArgs,
- EagerLoadModules, Format == ScanningOutputFormat::P1689);
+ Service, std::move(Opts), ScanInstance, Consumer, Controller,
+ OriginalInvocation, std::move(PrebuiltModuleVFSMap));
ScanInstance.addDependencyCollector(MDC);
break;
}
@@ -433,7 +429,7 @@ class DependencyScanningAction : public tooling::ToolAction {
std::unique_ptr<FrontendAction> Action;
- if (Format == ScanningOutputFormat::P1689)
+ if (Service.getFormat() == ScanningOutputFormat::P1689)
Action = std::make_unique<PreprocessOnlyAction>();
else if (ModuleName)
Action = std::make_unique<GetDependenciesByModuleNameAction>(*ModuleName);
@@ -476,14 +472,11 @@ class DependencyScanningAction : public tooling::ToolAction {
LastCC1Arguments = CI.getCC1CommandLine();
}
-private:
+ DependencyScanningService &Service;
StringRef WorkingDirectory;
DependencyConsumer &Consumer;
DependencyActionController &Controller;
llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
- ScanningOutputFormat Format;
- ScanningOptimizations OptimizeArgs;
- bool EagerLoadModules;
bool DisableFree;
std::optional<StringRef> ModuleName;
std::optional<CompilerInstance> ScanInstanceStorage;
@@ -498,8 +491,7 @@ class DependencyScanningAction : public tooling::ToolAction {
DependencyScanningWorker::DependencyScanningWorker(
DependencyScanningService &Service,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
- : Format(Service.getFormat()), OptimizeArgs(Service.getOptimizeArgs()),
- EagerLoadModules(Service.shouldEagerLoadModules()) {
+ : Service(Service) {
PCHContainerOps = std::make_shared<PCHContainerOperations>();
// We need to read object files from PCH built outside the scanner.
PCHContainerOps->registerReader(
@@ -655,9 +647,8 @@ bool DependencyScanningWorker::scanDependencies(
// in-process; preserve the original value, which is
// always true for a driver invocation.
bool DisableFree = true;
- DependencyScanningAction Action(WorkingDirectory, Consumer, Controller, DepFS,
- Format, OptimizeArgs, EagerLoadModules,
- DisableFree, ModuleName);
+ DependencyScanningAction Action(Service, WorkingDirectory, Consumer,
+ Controller, DepFS, DisableFree, ModuleName);
bool Success = false;
if (CommandLine[1] == "-cc1") {
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 7da3ec71f2da4..36b75c1016cd8 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -295,7 +295,8 @@ ModuleDepCollector::getInvocationAdjustedForModuleBuildWithoutOutputs(
// TODO: Verify this works fine when modulemap for module A is eagerly
// loaded from A.pcm, and module map passed on the command line contains
// definition of a submodule: "explicit module A.Private { ... }".
- if (EagerLoadModules && DepModuleMapFiles.contains(*ModuleMapEntry))
+ if (Service.shouldEagerLoadModules() &&
+ DepModuleMapFiles.contains(*ModuleMapEntry))
continue;
// Don't report module map file of the current module unless it also
@@ -345,7 +346,7 @@ llvm::DenseSet<const FileEntry *> ModuleDepCollector::collectModuleMapFiles(
void ModuleDepCollector::addModuleMapFiles(
CompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
- if (EagerLoadModules)
+ if (Service.shouldEagerLoadModules())
return; // Only pcm is needed for eager load.
for (const ModuleID &MID : ClangModuleDeps) {
@@ -360,7 +361,7 @@ void ModuleDepCollector::addModuleFiles(
for (const ModuleID &MID : ClangModuleDeps) {
std::string PCMPath =
Controller.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
- if (EagerLoadModules)
+ if (Service.shouldEagerLoadModules())
CI.getFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
else
CI.getHeaderSearchOpts().PrebuiltModuleFiles.insert(
@@ -373,7 +374,7 @@ void ModuleDepCollector::addModuleFiles(
for (const ModuleID &MID : ClangModuleDeps) {
std::string PCMPath =
Controller.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
- if (EagerLoadModules)
+ if (Service.shouldEagerLoadModules())
CI.getMutFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
else
CI.getMutHeaderSearchOpts().PrebuiltModuleFiles.insert(
@@ -551,8 +552,8 @@ static std::string getModuleContextHash(const ModuleDeps &MD,
void ModuleDepCollector::associateWithContextHash(
const CowCompilerInvocation &CI, bool IgnoreCWD, ModuleDeps &Deps) {
Deps.ID.ContextHash =
- getModuleContextHash(Deps, CI, EagerLoadModules, IgnoreCWD,
- ScanInstance.getVirtualFileSystem());
+ getModuleContextHash(Deps, CI, Service.shouldEagerLoadModules(),
+ IgnoreCWD, ScanInstance.getVirtualFileSystem());
bool Inserted = ModuleDepsByID.insert({Deps.ID, &Deps}).second;
(void)Inserted;
assert(Inserted && "duplicate module mapping");
@@ -656,7 +657,7 @@ void ModuleDepCollectorPP::EndOfMainFile() {
MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
- if (MDC.IsStdModuleP1689Format)
+ if (MDC.Service.getFormat() == ScanningOutputFormat::P1689)
MDC.Consumer.handleProvidedAndRequiredStdCXXModules(
MDC.ProvidedStdCXXModule, MDC.RequiredStdCXXModules);
@@ -753,21 +754,23 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
CowCompilerInvocation CI =
MDC.getInvocationAdjustedForModuleBuildWithoutOutputs(
MD, [&](CowCompilerInvocation &BuildInvocation) {
- if (any(MDC.OptimizeArgs & (ScanningOptimizations::HeaderSearch |
- ScanningOptimizations::VFS)))
+ if (any(MDC.Service.getOptimizeArgs() &
+ (ScanningOptimizations::HeaderSearch |
+ ScanningOptimizations::VFS)))
optimizeHeaderSearchOpts(BuildInvocation.getMutHeaderSearchOpts(),
*MDC.ScanInstance.getASTReader(), *MF,
MDC.PrebuiltModuleVFSMap,
- MDC.OptimizeArgs);
+ MDC.Service.getOptimizeArgs());
- if (any(MDC.OptimizeArgs & ScanningOptimizations::SystemWarnings))
+ if (any(MDC.Service.getOptimizeArgs() &
+ ScanningOptimizations::SystemWarnings))
optimizeDiagnosticOpts(
BuildInvocation.getMutDiagnosticOpts(),
BuildInvocation.getFrontendOpts().IsSystemModule);
- IgnoreCWD =
- any(MDC.OptimizeArgs & ScanningOptimizations::IgnoreCWD) &&
- isSafeToIgnoreCWD(BuildInvocation);
+ IgnoreCWD = any(MDC.Service.getOptimizeArgs() &
+ ScanningOptimizations::IgnoreCWD) &&
+ isSafeToIgnoreCWD(BuildInvocation);
if (IgnoreCWD) {
llvm::ErrorOr<std::string> CWD =
MDC.ScanInstance.getVirtualFileSystem()
@@ -870,19 +873,17 @@ void ModuleDepCollectorPP::addAffectingClangModule(
}
ModuleDepCollector::ModuleDepCollector(
+ DependencyScanningService &Service,
std::unique_ptr<DependencyOutputOptions> Opts,
CompilerInstance &ScanInstance, DependencyConsumer &C,
DependencyActionController &Controller, CompilerInvocation OriginalCI,
- PrebuiltModuleVFSMapT PrebuiltModuleVFSMap,
- ScanningOptimizations OptimizeArgs, bool EagerLoadModules,
- bool IsStdModuleP1689Format)
- : ScanInstance(ScanInstance), Consumer(C), Controller(Controller),
+ PrebuiltModuleVFSMapT PrebuiltModuleVFSMap)
+ : Service(Service), ScanInstance(ScanInstance), Consumer(C),
+ Controller(Controller),
PrebuiltModuleVFSMap(std::move(PrebuiltModuleVFSMap)),
Opts(std::move(Opts)),
CommonInvocation(
- makeCommonInvocationForModuleBuild(std::move(OriginalCI))),
- OptimizeArgs(OptimizeArgs), EagerLoadModules(EagerLoadModules),
- IsStdModuleP1689Format(IsStdModuleP1689Format) {}
+ makeCommonInvocationForModuleBuild(std::move(OriginalCI))) {}
void ModuleDepCollector::attachToPreprocessor(Preprocessor &PP) {
PP.addPPCallbacks(std::make_unique<ModuleDepCollectorPP>(*this));
@@ -914,7 +915,7 @@ static StringRef makeAbsoluteAndPreferred(CompilerInstance &CI, StringRef Path,
}
void ModuleDepCollector::addFileDep(StringRef Path) {
- if (IsStdModuleP1689Format) {
+ if (Service.getFormat() == ScanningOutputFormat::P1689) {
// Within P1689 format, we don't want all the paths to be absolute path
// since it may violate the traditional make style dependencies info.
FileDeps.emplace_back(Path);
>From dff02ab894d7c9ec7cc024a0eb198773df91684a Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Thu, 27 Feb 2025 09:13:51 -0800
Subject: [PATCH 2/2] Document service lifetime
---
.../clang/Tooling/DependencyScanning/DependencyScanningTool.h | 3 +++
.../Tooling/DependencyScanning/DependencyScanningWorker.h | 4 ++++
2 files changed, 7 insertions(+)
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index bcc9ea17e2588..d13c3ee76d74f 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -79,6 +79,9 @@ struct P1689Rule {
class DependencyScanningTool {
public:
/// Construct a dependency scanning tool.
+ ///
+ /// @param Service The parent service. Must outlive the tool.
+ /// @param FS The filesystem for the tool to use. Defaults to the physical FS.
DependencyScanningTool(DependencyScanningService &Service,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS =
llvm::vfs::createPhysicalFileSystem());
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index cd9d191e68aeb..5f0b983ebb58f 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -80,6 +80,10 @@ class DependencyActionController {
/// using the regular processing run.
class DependencyScanningWorker {
public:
+ /// Construct a dependency scanning worker.
+ ///
+ /// @param Service The parent service. Must outlive the worker.
+ /// @param FS The filesystem for the worker to use.
DependencyScanningWorker(DependencyScanningService &Service,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
More information about the cfe-commits
mailing list