Merge pull request swiftlang#10162 from swiftlang/jan_svoboda/stable-scanning-service

🍒 [clang][deps] Propagate the entire service

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.

(cherry picked from commit d2e6662)

Author: jansvoboda11

clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h

@@ -133,7 +133,7 @@ class DependencyScanningService {
   std::shared_ptr<llvm::cas::ObjectStore> CAS;
   std::shared_ptr<llvm::cas::ActionCache> Cache;
   /// Whether to optimize the modules' command-line arguments.
-  const ScanningOptimizations OptimizeArgs;
+  ScanningOptimizations OptimizeArgs;
   /// Whether to set up command-lines to load PCM files eagerly.
   const bool EagerLoadModules;
   /// Whether to trace VFS accesses.

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

@@ -96,6 +96,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());

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

@@ -116,6 +116,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);
 
@@ -177,7 +181,7 @@ class DependencyScanningWorker {
       DependencyActionController &Controller, DiagnosticConsumer &DiagsConsumer,
       raw_ostream *VerboseOS, bool DiagGenerationAsCompilation);
 
-  ScanningOutputFormat getScanningFormat() const { return Format; }
+  ScanningOutputFormat getScanningFormat() const { return Service.getFormat(); }
 
   CachingOnDiskFileSystemPtr getCASFS() { return CacheFS; }
   const CASOptions &getCASOpts() const { return CASOpts; }
@@ -188,11 +192,11 @@ class DependencyScanningWorker {
   /// each invocation.
   llvm::IntrusiveRefCntPtr<FileManager> getOrCreateFileManager() const;
 
-  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
@@ -202,11 +206,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;
 
   /// The caching file system.
   CachingOnDiskFileSystemPtr CacheFS;

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

@@ -237,13 +237,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;
@@ -255,6 +254,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.
@@ -286,13 +287,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;

clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Basic/BitmaskEnum.h"
 #include "llvm/CAS/ActionCache.h"
 #include "llvm/CAS/CachingOnDiskFileSystem.h"
 #include "llvm/CAS/ObjectStore.h"
@@ -28,6 +29,12 @@ DependencyScanningService::DependencyScanningService(
   if (!this->SharedFS)
     SharedCache.emplace();
 
+  // The FullIncludeTree output format completely subsumes header search and
+  // VFS optimizations due to how it works. Disable these optimizations so
+  // we're not doing unneeded work.
+  if (Format == ScanningOutputFormat::FullIncludeTree)
+    this->OptimizeArgs &= ~ScanningOptimizations::FullIncludeTreeIrrelevant;
+
   // Initialize targets for object file support.
   llvm::InitializeAllTargets();
   llvm::InitializeAllTargetMCs();

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

@@ -394,30 +394,22 @@ class WrapScanModuleBuildAction : public WrapperFrontendAction {
 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,
       llvm::IntrusiveRefCntPtr<DependencyScanningCASFilesystem> DepCASFS,
       llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
-      ScanningOutputFormat Format, ScanningOptimizations OptimizeArgs,
-      bool EagerLoadModules, bool DisableFree, bool EmitDependencyFile,
+      bool DisableFree, bool EmitDependencyFile,
       bool DiagGenerationAsCompilation, const CASOptions &CASOpts,
       std::optional<StringRef> ModuleName = std::nullopt,
       raw_ostream *VerboseOS = nullptr)
-      : WorkingDirectory(WorkingDirectory), Consumer(Consumer),
+      : Service(Service), WorkingDirectory(WorkingDirectory), Consumer(Consumer),
         Controller(Controller), DepFS(std::move(DepFS)),
         DepCASFS(std::move(DepCASFS)), CacheFS(std::move(CacheFS)),
-        Format(Format), OptimizeArgs(OptimizeArgs),
-        EagerLoadModules(EagerLoadModules), DisableFree(DisableFree),
+        DisableFree(DisableFree),
         CASOpts(CASOpts), EmitDependencyFile(EmitDependencyFile),
         DiagGenerationAsCompilation(DiagGenerationAsCompilation),
-        ModuleName(ModuleName), VerboseOS(VerboseOS) {
-    // The FullIncludeTree output format completely subsumes header search and
-    // VFS optimizations due to how it works. Disable these optimizations so
-    // we're not doing unneeded work.
-    if (Format == ScanningOutputFormat::FullIncludeTree)
-      this->OptimizeArgs &= ~ScanningOptimizations::FullIncludeTreeIrrelevant;
-  }
+        ModuleName(ModuleName), VerboseOS(VerboseOS) {}
 
   bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
                      FileManager *DriverFileMgr,
@@ -427,7 +419,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) {
@@ -478,7 +470,7 @@ class DependencyScanningAction : public tooling::ToolAction {
       ScanInstance.getFrontendOpts().ModulesShareFileManager = false;
     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(
@@ -535,7 +527,7 @@ class DependencyScanningAction : public tooling::ToolAction {
       Opts->Targets = {
           deduceDepTarget(ScanInstance.getFrontendOpts().OutputFile,
                           ScanInstance.getFrontendOpts().Inputs)};
-    if (Format == ScanningOutputFormat::Make) {
+    if (Service.getFormat() == ScanningOutputFormat::Make) {
       // Only 'Make' scanning needs to force this because that mode depends on
       // getting the dependencies directly from \p DependencyFileGenerator.
       Opts->IncludeSystemHeaders = true;
@@ -555,7 +547,7 @@ class DependencyScanningAction : public tooling::ToolAction {
     // \p DependencyScanningAction, and have the callers pass in a
     // “DependencyCollector factory” so the connection of collector<->consumer
     // is explicit in each \p DependencyScanningTool function.
-    switch (Format) {
+    switch (Service.getFormat()) {
     case ScanningOutputFormat::Make:
     case ScanningOutputFormat::Tree:
       ScanInstance.addDependencyCollector(
@@ -576,9 +568,8 @@ class DependencyScanningAction : public tooling::ToolAction {
       }
 
       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);
       ScanInstance.setGenModuleActionWrapper(
           [&Controller = Controller](const FrontendOptions &Opts,
@@ -680,16 +671,13 @@ class DependencyScanningAction : public tooling::ToolAction {
     return nullptr;
   }
 
-private:
+  DependencyScanningService &Service;
   StringRef WorkingDirectory;
   DependencyConsumer &Consumer;
   DependencyActionController &Controller;
   llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
   llvm::IntrusiveRefCntPtr<DependencyScanningCASFilesystem> DepCASFS;
   llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS;
-  ScanningOutputFormat Format;
-  ScanningOptimizations OptimizeArgs;
-  bool EagerLoadModules;
   bool DisableFree;
   const CASOptions &CASOpts;
   bool EmitDependencyFile = false;
@@ -708,8 +696,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),
       CASOpts(Service.getCASOpts()), CAS(Service.getCAS()) {
   PCHContainerOps = std::make_shared<PCHContainerOperations>();
   // We need to read object files from PCH built outside the scanner.
@@ -878,9 +865,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,
+  DependencyScanningAction Action(Service, WorkingDirectory, Consumer, Controller, DepFS,
                                   DepCASFS, CacheFS,
-                                  Format, OptimizeArgs, EagerLoadModules,
                                   DisableFree,
                                   /*EmitDependencyFile=*/false,
                                   /*DiagGenerationAsCompilation=*/false, getCASOpts(),
@@ -1033,13 +1019,12 @@ void DependencyScanningWorker::computeDependenciesFromCompilerInvocation(
 
   // FIXME: EmitDependencyFile should only be set when it's for a real
   // compilation.
-  DependencyScanningAction Action(
-      WorkingDirectory, DepsConsumer, Controller, DepFS, DepCASFS, CacheFS,
-      Format,
-      ScanningOptimizations::Default, /*DisableFree=*/false, EagerLoadModules,
-      /*EmitDependencyFile=*/!DepFile.empty(), DiagGenerationAsCompilation,
-      getCASOpts(),
-      /*ModuleName=*/std::nullopt, VerboseOS);
+  DependencyScanningAction Action(Service, WorkingDirectory, DepsConsumer,
+                                  Controller, DepFS, DepCASFS, CacheFS,
+                                  /*DisableFree=*/false,
+                                  /*EmitDependencyFile=*/!DepFile.empty(),
+                                  DiagGenerationAsCompilation, getCASOpts(),
+                                  /*ModuleName=*/std::nullopt, VerboseOS);
 
   // Ignore result; we're just collecting dependencies.
   //

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

@@ -307,7 +307,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
@@ -360,7 +361,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) {
@@ -382,7 +383,7 @@ void ModuleDepCollector::addModuleFiles(
       CI.getFrontendOpts().ModuleCacheKeys.emplace_back(PCMPath,
                                                         *MD->ModuleCacheKey);
 
-    if (EagerLoadModules)
+    if (Service.shouldEagerLoadModules())
       CI.getFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
     else
       CI.getHeaderSearchOpts().PrebuiltModuleFiles.insert(
@@ -402,7 +403,7 @@ void ModuleDepCollector::addModuleFiles(
       CI.getMutFrontendOpts().ModuleCacheKeys.emplace_back(PCMPath,
                                                            *MD->ModuleCacheKey);
 
-    if (EagerLoadModules)
+    if (Service.shouldEagerLoadModules())
       CI.getMutFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
     else
       CI.getMutHeaderSearchOpts().PrebuiltModuleFiles.insert(
@@ -622,8 +623,8 @@ static void checkCompileCacheKeyMatch(cas::ObjectStore &CAS,
 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");
@@ -728,7 +729,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);
 
@@ -838,21 +839,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()
@@ -977,19 +980,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));
@@ -1021,7 +1022,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);