Skip to content

Commit d2e6662

Browse files
jansvoboda11jansvoboda11
authored
[clang][deps] Propagate the entire service (llvm#128959)
Shared state between dependency scanning workers is managed by the dependency scanning service. Right now, the members are individually threaded through the worker, action, and collector. This makes any change to the service and its members a very laborious process. Moreover, this situation causes frequent merge conflicts in our downstream repo where the service does have some extra members that need to be passed around. To ease the maintenance burden, this PR starts passing a reference to the entire service.
1 parent f6bfa33 commit d2e6662

File tree

5 files changed

+52
-64
lines changed

5 files changed

+52
-64
lines changed

‎clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ struct P1689Rule {
7979
class DependencyScanningTool {
8080
public:
8181
/// Construct a dependency scanning tool.
82+
///
83+
/// @param Service The parent service. Must outlive the tool.
84+
/// @param FS The filesystem for the tool to use. Defaults to the physical FS.
8285
DependencyScanningTool(DependencyScanningService &Service,
8386
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS =
8487
llvm::vfs::createPhysicalFileSystem());

‎clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ class DependencyActionController {
8080
/// using the regular processing run.
8181
class DependencyScanningWorker {
8282
public:
83+
/// Construct a dependency scanning worker.
84+
///
85+
/// @param Service The parent service. Must outlive the worker.
86+
/// @param FS The filesystem for the worker to use.
8387
DependencyScanningWorker(DependencyScanningService &Service,
8488
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
8589

@@ -130,11 +134,11 @@ class DependencyScanningWorker {
130134
DependencyActionController &Controller,
131135
StringRef ModuleName);
132136

133-
bool shouldEagerLoadModules() const { return EagerLoadModules; }
134-
135137
llvm::vfs::FileSystem &getVFS() const { return *BaseFS; }
136138

137139
private:
140+
/// The parent dependency scanning service.
141+
DependencyScanningService &Service;
138142
std::shared_ptr<PCHContainerOperations> PCHContainerOps;
139143
/// The file system to be used during the scan.
140144
/// This is either \c FS passed in the constructor (when performing canonical
@@ -144,11 +148,6 @@ class DependencyScanningWorker {
144148
/// dependency-directives-extracting) filesystem overlaid on top of \c FS
145149
/// (passed in the constructor).
146150
llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
147-
ScanningOutputFormat Format;
148-
/// Whether to optimize the modules' command-line arguments.
149-
ScanningOptimizations OptimizeArgs;
150-
/// Whether to set up command-lines to load PCM files eagerly.
151-
bool EagerLoadModules;
152151

153152
/// Private helper functions.
154153
bool scanDependencies(StringRef WorkingDirectory,

‎clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,12 @@ class ModuleDepCollectorPP final : public PPCallbacks {
225225
/// \c ModuleDepCollectorPP to the preprocessor.
226226
class ModuleDepCollector final : public DependencyCollector {
227227
public:
228-
ModuleDepCollector(std::unique_ptr<DependencyOutputOptions> Opts,
228+
ModuleDepCollector(DependencyScanningService &Service,
229+
std::unique_ptr<DependencyOutputOptions> Opts,
229230
CompilerInstance &ScanInstance, DependencyConsumer &C,
230231
DependencyActionController &Controller,
231232
CompilerInvocation OriginalCI,
232-
PrebuiltModuleVFSMapT PrebuiltModuleVFSMap,
233-
ScanningOptimizations OptimizeArgs, bool EagerLoadModules,
234-
bool IsStdModuleP1689Format);
233+
PrebuiltModuleVFSMapT PrebuiltModuleVFSMap);
235234

236235
void attachToPreprocessor(Preprocessor &PP) override;
237236
void attachToASTReader(ASTReader &R) override;
@@ -243,6 +242,8 @@ class ModuleDepCollector final : public DependencyCollector {
243242
private:
244243
friend ModuleDepCollectorPP;
245244

245+
/// The parent dependency scanning service.
246+
DependencyScanningService &Service;
246247
/// The compiler instance for scanning the current translation unit.
247248
CompilerInstance &ScanInstance;
248249
/// The consumer of collected dependency information.
@@ -274,13 +275,6 @@ class ModuleDepCollector final : public DependencyCollector {
274275
/// a discovered modular dependency. Note that this still needs to be adjusted
275276
/// for each individual module.
276277
CowCompilerInvocation CommonInvocation;
277-
/// Whether to optimize the modules' command-line arguments.
278-
ScanningOptimizations OptimizeArgs;
279-
/// Whether to set up command-lines to load PCM files eagerly.
280-
bool EagerLoadModules;
281-
/// If we're generating dependency output in P1689 format
282-
/// for standard C++ modules.
283-
bool IsStdModuleP1689Format;
284278

285279
std::optional<P1689ModuleInfo> ProvidedStdCXXModule;
286280
std::vector<P1689ModuleInfo> RequiredStdCXXModules;

‎clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp‎

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -284,15 +284,12 @@ static void canonicalizeDefines(PreprocessorOptions &PPOpts) {
284284
class DependencyScanningAction : public tooling::ToolAction {
285285
public:
286286
DependencyScanningAction(
287-
StringRef WorkingDirectory, DependencyConsumer &Consumer,
288-
DependencyActionController &Controller,
287+
DependencyScanningService &Service, StringRef WorkingDirectory,
288+
DependencyConsumer &Consumer, DependencyActionController &Controller,
289289
llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS,
290-
ScanningOutputFormat Format, ScanningOptimizations OptimizeArgs,
291-
bool EagerLoadModules, bool DisableFree,
292-
std::optional<StringRef> ModuleName = std::nullopt)
293-
: WorkingDirectory(WorkingDirectory), Consumer(Consumer),
294-
Controller(Controller), DepFS(std::move(DepFS)), Format(Format),
295-
OptimizeArgs(OptimizeArgs), EagerLoadModules(EagerLoadModules),
290+
bool DisableFree, std::optional<StringRef> ModuleName = std::nullopt)
291+
: Service(Service), WorkingDirectory(WorkingDirectory),
292+
Consumer(Consumer), Controller(Controller), DepFS(std::move(DepFS)),
296293
DisableFree(DisableFree), ModuleName(ModuleName) {}
297294

298295
bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
@@ -303,7 +300,7 @@ class DependencyScanningAction : public tooling::ToolAction {
303300
CompilerInvocation OriginalInvocation(*Invocation);
304301
// Restore the value of DisableFree, which may be modified by Tooling.
305302
OriginalInvocation.getFrontendOpts().DisableFree = DisableFree;
306-
if (any(OptimizeArgs & ScanningOptimizations::Macros))
303+
if (any(Service.getOptimizeArgs() & ScanningOptimizations::Macros))
307304
canonicalizeDefines(OriginalInvocation.getPreprocessorOpts());
308305

309306
if (Scanned) {
@@ -340,7 +337,7 @@ class DependencyScanningAction : public tooling::ToolAction {
340337
ScanInstance.getFrontendOpts().ModulesShareFileManager = true;
341338
ScanInstance.getHeaderSearchOpts().ModuleFormat = "raw";
342339
ScanInstance.getHeaderSearchOpts().ModulesIncludeVFSUsage =
343-
any(OptimizeArgs & ScanningOptimizations::VFS);
340+
any(Service.getOptimizeArgs() & ScanningOptimizations::VFS);
344341

345342
// Support for virtual file system overlays.
346343
auto FS = createVFSFromCompilerInvocation(
@@ -400,7 +397,7 @@ class DependencyScanningAction : public tooling::ToolAction {
400397
ScanInstance.getFrontendOpts().Inputs)};
401398
Opts->IncludeSystemHeaders = true;
402399

403-
switch (Format) {
400+
switch (Service.getFormat()) {
404401
case ScanningOutputFormat::Make:
405402
ScanInstance.addDependencyCollector(
406403
std::make_shared<DependencyConsumerForwarder>(
@@ -409,9 +406,8 @@ class DependencyScanningAction : public tooling::ToolAction {
409406
case ScanningOutputFormat::P1689:
410407
case ScanningOutputFormat::Full:
411408
MDC = std::make_shared<ModuleDepCollector>(
412-
std::move(Opts), ScanInstance, Consumer, Controller,
413-
OriginalInvocation, std::move(PrebuiltModuleVFSMap), OptimizeArgs,
414-
EagerLoadModules, Format == ScanningOutputFormat::P1689);
409+
Service, std::move(Opts), ScanInstance, Consumer, Controller,
410+
OriginalInvocation, std::move(PrebuiltModuleVFSMap));
415411
ScanInstance.addDependencyCollector(MDC);
416412
break;
417413
}
@@ -433,7 +429,7 @@ class DependencyScanningAction : public tooling::ToolAction {
433429

434430
std::unique_ptr<FrontendAction> Action;
435431

436-
if (Format == ScanningOutputFormat::P1689)
432+
if (Service.getFormat() == ScanningOutputFormat::P1689)
437433
Action = std::make_unique<PreprocessOnlyAction>();
438434
else if (ModuleName)
439435
Action = std::make_unique<GetDependenciesByModuleNameAction>(*ModuleName);
@@ -476,14 +472,11 @@ class DependencyScanningAction : public tooling::ToolAction {
476472
LastCC1Arguments = CI.getCC1CommandLine();
477473
}
478474

479-
private:
475+
DependencyScanningService &Service;
480476
StringRef WorkingDirectory;
481477
DependencyConsumer &Consumer;
482478
DependencyActionController &Controller;
483479
llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
484-
ScanningOutputFormat Format;
485-
ScanningOptimizations OptimizeArgs;
486-
bool EagerLoadModules;
487480
bool DisableFree;
488481
std::optional<StringRef> ModuleName;
489482
std::optional<CompilerInstance> ScanInstanceStorage;
@@ -498,8 +491,7 @@ class DependencyScanningAction : public tooling::ToolAction {
498491
DependencyScanningWorker::DependencyScanningWorker(
499492
DependencyScanningService &Service,
500493
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
501-
: Format(Service.getFormat()), OptimizeArgs(Service.getOptimizeArgs()),
502-
EagerLoadModules(Service.shouldEagerLoadModules()) {
494+
: Service(Service) {
503495
PCHContainerOps = std::make_shared<PCHContainerOperations>();
504496
// We need to read object files from PCH built outside the scanner.
505497
PCHContainerOps->registerReader(
@@ -655,9 +647,8 @@ bool DependencyScanningWorker::scanDependencies(
655647
// in-process; preserve the original value, which is
656648
// always true for a driver invocation.
657649
bool DisableFree = true;
658-
DependencyScanningAction Action(WorkingDirectory, Consumer, Controller, DepFS,
659-
Format, OptimizeArgs, EagerLoadModules,
660-
DisableFree, ModuleName);
650+
DependencyScanningAction Action(Service, WorkingDirectory, Consumer,
651+
Controller, DepFS, DisableFree, ModuleName);
661652

662653
bool Success = false;
663654
if (CommandLine[1] == "-cc1") {

‎clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,8 @@ ModuleDepCollector::getInvocationAdjustedForModuleBuildWithoutOutputs(
295295
// TODO: Verify this works fine when modulemap for module A is eagerly
296296
// loaded from A.pcm, and module map passed on the command line contains
297297
// definition of a submodule: "explicit module A.Private { ... }".
298-
if (EagerLoadModules && DepModuleMapFiles.contains(*ModuleMapEntry))
298+
if (Service.shouldEagerLoadModules() &&
299+
DepModuleMapFiles.contains(*ModuleMapEntry))
299300
continue;
300301

301302
// Don't report module map file of the current module unless it also
@@ -345,7 +346,7 @@ llvm::DenseSet<const FileEntry *> ModuleDepCollector::collectModuleMapFiles(
345346

346347
void ModuleDepCollector::addModuleMapFiles(
347348
CompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
348-
if (EagerLoadModules)
349+
if (Service.shouldEagerLoadModules())
349350
return; // Only pcm is needed for eager load.
350351

351352
for (const ModuleID &MID : ClangModuleDeps) {
@@ -360,7 +361,7 @@ void ModuleDepCollector::addModuleFiles(
360361
for (const ModuleID &MID : ClangModuleDeps) {
361362
std::string PCMPath =
362363
Controller.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
363-
if (EagerLoadModules)
364+
if (Service.shouldEagerLoadModules())
364365
CI.getFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
365366
else
366367
CI.getHeaderSearchOpts().PrebuiltModuleFiles.insert(
@@ -373,7 +374,7 @@ void ModuleDepCollector::addModuleFiles(
373374
for (const ModuleID &MID : ClangModuleDeps) {
374375
std::string PCMPath =
375376
Controller.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
376-
if (EagerLoadModules)
377+
if (Service.shouldEagerLoadModules())
377378
CI.getMutFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
378379
else
379380
CI.getMutHeaderSearchOpts().PrebuiltModuleFiles.insert(
@@ -551,8 +552,8 @@ static std::string getModuleContextHash(const ModuleDeps &MD,
551552
void ModuleDepCollector::associateWithContextHash(
552553
const CowCompilerInvocation &CI, bool IgnoreCWD, ModuleDeps &Deps) {
553554
Deps.ID.ContextHash =
554-
getModuleContextHash(Deps, CI, EagerLoadModules, IgnoreCWD,
555-
ScanInstance.getVirtualFileSystem());
555+
getModuleContextHash(Deps, CI, Service.shouldEagerLoadModules(),
556+
IgnoreCWD, ScanInstance.getVirtualFileSystem());
556557
bool Inserted = ModuleDepsByID.insert({Deps.ID, &Deps}).second;
557558
(void)Inserted;
558559
assert(Inserted && "duplicate module mapping");
@@ -656,7 +657,7 @@ void ModuleDepCollectorPP::EndOfMainFile() {
656657

657658
MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
658659

659-
if (MDC.IsStdModuleP1689Format)
660+
if (MDC.Service.getFormat() == ScanningOutputFormat::P1689)
660661
MDC.Consumer.handleProvidedAndRequiredStdCXXModules(
661662
MDC.ProvidedStdCXXModule, MDC.RequiredStdCXXModules);
662663

@@ -753,21 +754,23 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
753754
CowCompilerInvocation CI =
754755
MDC.getInvocationAdjustedForModuleBuildWithoutOutputs(
755756
MD, [&](CowCompilerInvocation &BuildInvocation) {
756-
if (any(MDC.OptimizeArgs & (ScanningOptimizations::HeaderSearch |
757-
ScanningOptimizations::VFS)))
757+
if (any(MDC.Service.getOptimizeArgs() &
758+
(ScanningOptimizations::HeaderSearch |
759+
ScanningOptimizations::VFS)))
758760
optimizeHeaderSearchOpts(BuildInvocation.getMutHeaderSearchOpts(),
759761
*MDC.ScanInstance.getASTReader(), *MF,
760762
MDC.PrebuiltModuleVFSMap,
761-
MDC.OptimizeArgs);
763+
MDC.Service.getOptimizeArgs());
762764

763-
if (any(MDC.OptimizeArgs & ScanningOptimizations::SystemWarnings))
765+
if (any(MDC.Service.getOptimizeArgs() &
766+
ScanningOptimizations::SystemWarnings))
764767
optimizeDiagnosticOpts(
765768
BuildInvocation.getMutDiagnosticOpts(),
766769
BuildInvocation.getFrontendOpts().IsSystemModule);
767770

768-
IgnoreCWD =
769-
any(MDC.OptimizeArgs & ScanningOptimizations::IgnoreCWD) &&
770-
isSafeToIgnoreCWD(BuildInvocation);
771+
IgnoreCWD = any(MDC.Service.getOptimizeArgs() &
772+
ScanningOptimizations::IgnoreCWD) &&
773+
isSafeToIgnoreCWD(BuildInvocation);
771774
if (IgnoreCWD) {
772775
llvm::ErrorOr<std::string> CWD =
773776
MDC.ScanInstance.getVirtualFileSystem()
@@ -870,19 +873,17 @@ void ModuleDepCollectorPP::addAffectingClangModule(
870873
}
871874

872875
ModuleDepCollector::ModuleDepCollector(
876+
DependencyScanningService &Service,
873877
std::unique_ptr<DependencyOutputOptions> Opts,
874878
CompilerInstance &ScanInstance, DependencyConsumer &C,
875879
DependencyActionController &Controller, CompilerInvocation OriginalCI,
876-
PrebuiltModuleVFSMapT PrebuiltModuleVFSMap,
877-
ScanningOptimizations OptimizeArgs, bool EagerLoadModules,
878-
bool IsStdModuleP1689Format)
879-
: ScanInstance(ScanInstance), Consumer(C), Controller(Controller),
880+
PrebuiltModuleVFSMapT PrebuiltModuleVFSMap)
881+
: Service(Service), ScanInstance(ScanInstance), Consumer(C),
882+
Controller(Controller),
880883
PrebuiltModuleVFSMap(std::move(PrebuiltModuleVFSMap)),
881884
Opts(std::move(Opts)),
882885
CommonInvocation(
883-
makeCommonInvocationForModuleBuild(std::move(OriginalCI))),
884-
OptimizeArgs(OptimizeArgs), EagerLoadModules(EagerLoadModules),
885-
IsStdModuleP1689Format(IsStdModuleP1689Format) {}
886+
makeCommonInvocationForModuleBuild(std::move(OriginalCI))) {}
886887

887888
void ModuleDepCollector::attachToPreprocessor(Preprocessor &PP) {
888889
PP.addPPCallbacks(std::make_unique<ModuleDepCollectorPP>(*this));
@@ -914,7 +915,7 @@ static StringRef makeAbsoluteAndPreferred(CompilerInstance &CI, StringRef Path,
914915
}
915916

916917
void ModuleDepCollector::addFileDep(StringRef Path) {
917-
if (IsStdModuleP1689Format) {
918+
if (Service.getFormat() == ScanningOutputFormat::P1689) {
918919
// Within P1689 format, we don't want all the paths to be absolute path
919920
// since it may violate the traditional make style dependencies info.
920921
FileDeps.emplace_back(Path);

0 commit comments

Comments
 (0)