benlangmuir created this revision.
benlangmuir added reviewers: jansvoboda11, akyrtzi, steven_wu.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The idea is to split the callbacks that are used to consume dependency
information (DependencyConsumer) from callbacks that modify the scan
behaviour itself in any way (DependencyActions). Currently this is just
lookupModuleOutput, but we have additional callbacks related to CAS
support that we intend to upstream in the future.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144058

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -56,16 +56,16 @@
 void ModuleDepCollector::addOutputPaths(CompilerInvocation &CI,
                                         ModuleDeps &Deps) {
   CI.getFrontendOpts().OutputFile =
-      Consumer.lookupModuleOutput(Deps.ID, ModuleOutputKind::ModuleFile);
+      Actions.lookupModuleOutput(Deps.ID, ModuleOutputKind::ModuleFile);
   if (!CI.getDiagnosticOpts().DiagnosticSerializationFile.empty())
     CI.getDiagnosticOpts().DiagnosticSerializationFile =
-        Consumer.lookupModuleOutput(
+        Actions.lookupModuleOutput(
             Deps.ID, ModuleOutputKind::DiagnosticSerializationFile);
   if (!CI.getDependencyOutputOpts().OutputFile.empty()) {
     CI.getDependencyOutputOpts().OutputFile =
-        Consumer.lookupModuleOutput(Deps.ID, ModuleOutputKind::DependencyFile);
+        Actions.lookupModuleOutput(Deps.ID, ModuleOutputKind::DependencyFile);
     CI.getDependencyOutputOpts().Targets =
-        splitString(Consumer.lookupModuleOutput(
+        splitString(Actions.lookupModuleOutput(
                         Deps.ID, ModuleOutputKind::DependencyTargets),
                     '\0');
     if (!CI.getDependencyOutputOpts().OutputFile.empty() &&
@@ -205,7 +205,7 @@
     CompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
   for (const ModuleID &MID : ClangModuleDeps) {
     std::string PCMPath =
-        Consumer.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
+        Actions.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
     if (EagerLoadModules)
       CI.getFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
     else
@@ -577,11 +577,11 @@
 ModuleDepCollector::ModuleDepCollector(
     std::unique_ptr<DependencyOutputOptions> Opts,
     CompilerInstance &ScanInstance, DependencyConsumer &C,
-    CompilerInvocation OriginalCI, bool OptimizeArgs, bool EagerLoadModules,
-    bool IsStdModuleP1689Format)
-    : ScanInstance(ScanInstance), Consumer(C), Opts(std::move(Opts)),
-      OriginalInvocation(std::move(OriginalCI)), OptimizeArgs(OptimizeArgs),
-      EagerLoadModules(EagerLoadModules),
+    DependencyActions &Actions, CompilerInvocation OriginalCI,
+    bool OptimizeArgs, bool EagerLoadModules, bool IsStdModuleP1689Format)
+    : ScanInstance(ScanInstance), Consumer(C), Actions(Actions),
+      Opts(std::move(Opts)), OriginalInvocation(std::move(OriginalCI)),
+      OptimizeArgs(OptimizeArgs), EagerLoadModules(EagerLoadModules),
       IsStdModuleP1689Format(IsStdModuleP1689Format) {}
 
 void ModuleDepCollector::attachToPreprocessor(Preprocessor &PP) {
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -146,13 +146,14 @@
 public:
   DependencyScanningAction(
       StringRef WorkingDirectory, DependencyConsumer &Consumer,
+      DependencyActions &Actions,
       llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS,
       ScanningOutputFormat Format, bool OptimizeArgs, bool EagerLoadModules,
       bool DisableFree, std::optional<StringRef> ModuleName = std::nullopt)
       : WorkingDirectory(WorkingDirectory), Consumer(Consumer),
-        DepFS(std::move(DepFS)), Format(Format), OptimizeArgs(OptimizeArgs),
-        EagerLoadModules(EagerLoadModules), DisableFree(DisableFree),
-        ModuleName(ModuleName) {}
+        Actions(Actions), DepFS(std::move(DepFS)), Format(Format),
+        OptimizeArgs(OptimizeArgs), EagerLoadModules(EagerLoadModules),
+        DisableFree(DisableFree), ModuleName(ModuleName) {}
 
   bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
                      FileManager *FileMgr,
@@ -250,7 +251,7 @@
     case ScanningOutputFormat::P1689:
     case ScanningOutputFormat::Full:
       MDC = std::make_shared<ModuleDepCollector>(
-          std::move(Opts), ScanInstance, Consumer, OriginalInvocation,
+          std::move(Opts), ScanInstance, Consumer, Actions, OriginalInvocation,
           OptimizeArgs, EagerLoadModules,
           Format == ScanningOutputFormat::P1689);
       ScanInstance.addDependencyCollector(MDC);
@@ -300,6 +301,7 @@
 private:
   StringRef WorkingDirectory;
   DependencyConsumer &Consumer;
+  DependencyActions &Actions;
   llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
   ScanningOutputFormat Format;
   bool OptimizeArgs;
@@ -342,7 +344,8 @@
 
 llvm::Error DependencyScanningWorker::computeDependencies(
     StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
-    DependencyConsumer &Consumer, std::optional<StringRef> ModuleName) {
+    DependencyConsumer &Consumer, DependencyActions &Actions,
+    std::optional<StringRef> ModuleName) {
   std::vector<const char *> CLI;
   for (const std::string &Arg : CommandLine)
     CLI.push_back(Arg.c_str());
@@ -355,8 +358,8 @@
   llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput);
   TextDiagnosticPrinter DiagPrinter(DiagnosticsOS, DiagOpts.release());
 
-  if (computeDependencies(WorkingDirectory, CommandLine, Consumer, DiagPrinter,
-                          ModuleName))
+  if (computeDependencies(WorkingDirectory, CommandLine, Consumer, Actions,
+                          DiagPrinter, ModuleName))
     return llvm::Error::success();
   return llvm::make_error<llvm::StringError>(DiagnosticsOS.str(),
                                              llvm::inconvertibleErrorCode());
@@ -388,8 +391,8 @@
 
 bool DependencyScanningWorker::computeDependencies(
     StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
-    DependencyConsumer &Consumer, DiagnosticConsumer &DC,
-    std::optional<StringRef> ModuleName) {
+    DependencyConsumer &Consumer, DependencyActions &Actions,
+    DiagnosticConsumer &DC, std::optional<StringRef> ModuleName) {
   // Reset what might have been modified in the previous worker invocation.
   BaseFS->setCurrentWorkingDirectory(WorkingDirectory);
 
@@ -445,9 +448,9 @@
   // in-process; preserve the original value, which is
   // always true for a driver invocation.
   bool DisableFree = true;
-  DependencyScanningAction Action(WorkingDirectory, Consumer, DepFS, Format,
-                                  OptimizeArgs, EagerLoadModules, DisableFree,
-                                  ModuleName);
+  DependencyScanningAction Action(WorkingDirectory, Consumer, Actions, DepFS,
+                                  Format, OptimizeArgs, EagerLoadModules,
+                                  DisableFree, ModuleName);
   bool Success = forEachDriverJob(
       FinalCommandLine, *Diags, *FileMgr, [&](const driver::Command &Cmd) {
         if (StringRef(Cmd.getCreator().getName()) != "clang") {
@@ -485,3 +488,5 @@
         << llvm::join(FinalCommandLine, " ");
   return Success && Action.hasScanned();
 }
+
+DependencyActions::~DependencyActions() {}
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -46,11 +46,6 @@
 
   void handleContextHash(std::string Hash) override {}
 
-  std::string lookupModuleOutput(const ModuleID &ID,
-                                 ModuleOutputKind Kind) override {
-    llvm::report_fatal_error("unexpected call to lookupModuleOutput");
-  }
-
   void printDependencies(std::string &S) {
     assert(Opts && "Handled dependency output options.");
 
@@ -77,12 +72,21 @@
   std::unique_ptr<DependencyOutputOptions> Opts;
   std::vector<std::string> Dependencies;
 };
+
+class NoModuleOutputsActions : public DependencyActions {
+public:
+  std::string lookupModuleOutput(const ModuleID &ID,
+                                 ModuleOutputKind Kind) override {
+    llvm::report_fatal_error("unexpected call to lookupModuleOutput");
+  }
+};
 } // anonymous namespace
 
 llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
     const std::vector<std::string> &CommandLine, StringRef CWD) {
   MakeDependencyPrinterConsumer Consumer;
-  auto Result = Worker.computeDependencies(CWD, CommandLine, Consumer);
+  NoModuleOutputsActions Actions;
+  auto Result = Worker.computeDependencies(CWD, CommandLine, Consumer, Actions);
   if (Result)
     return std::move(Result);
   std::string Output;
@@ -124,7 +128,9 @@
 
   P1689Rule Rule;
   P1689ModuleDependencyPrinterConsumer Consumer(Rule, Command);
-  auto Result = Worker.computeDependencies(CWD, Command.CommandLine, Consumer);
+  NoModuleOutputsActions Actions;
+  auto Result =
+      Worker.computeDependencies(CWD, Command.CommandLine, Consumer, Actions);
   if (Result)
     return std::move(Result);
 
@@ -139,8 +145,10 @@
     const std::vector<std::string> &CommandLine, StringRef CWD,
     const llvm::StringSet<> &AlreadySeen,
     LookupModuleOutputCallback LookupModuleOutput) {
-  FullDependencyConsumer Consumer(AlreadySeen, LookupModuleOutput);
-  llvm::Error Result = Worker.computeDependencies(CWD, CommandLine, Consumer);
+  FullDependencyConsumer Consumer(AlreadySeen);
+  CallbackDependencyActions Actions(LookupModuleOutput);
+  llvm::Error Result =
+      Worker.computeDependencies(CWD, CommandLine, Consumer, Actions);
   if (Result)
     return std::move(Result);
   return Consumer.takeTranslationUnitDeps();
@@ -150,9 +158,10 @@
     StringRef ModuleName, const std::vector<std::string> &CommandLine,
     StringRef CWD, const llvm::StringSet<> &AlreadySeen,
     LookupModuleOutputCallback LookupModuleOutput) {
-  FullDependencyConsumer Consumer(AlreadySeen, LookupModuleOutput);
-  llvm::Error Result =
-      Worker.computeDependencies(CWD, CommandLine, Consumer, ModuleName);
+  FullDependencyConsumer Consumer(AlreadySeen);
+  CallbackDependencyActions Actions(LookupModuleOutput);
+  llvm::Error Result = Worker.computeDependencies(CWD, CommandLine, Consumer,
+                                                  Actions, ModuleName);
   if (Result)
     return std::move(Result);
   return Consumer.takeModuleGraphDeps();
@@ -194,3 +203,5 @@
 
   return ModuleGraph;
 }
+
+CallbackDependencyActions::~CallbackDependencyActions() {}
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -27,6 +27,7 @@
 namespace tooling {
 namespace dependencies {
 
+class DependencyActions;
 class DependencyConsumer;
 
 /// Modular dependency that has already been built prior to the dependency scan.
@@ -201,8 +202,9 @@
 public:
   ModuleDepCollector(std::unique_ptr<DependencyOutputOptions> Opts,
                      CompilerInstance &ScanInstance, DependencyConsumer &C,
-                     CompilerInvocation OriginalCI, bool OptimizeArgs,
-                     bool EagerLoadModules, bool IsStdModuleP1689Format);
+                     DependencyActions &Actions, CompilerInvocation OriginalCI,
+                     bool OptimizeArgs, bool EagerLoadModules,
+                     bool IsStdModuleP1689Format);
 
   void attachToPreprocessor(Preprocessor &PP) override;
   void attachToASTReader(ASTReader &R) override;
@@ -218,6 +220,8 @@
   CompilerInstance &ScanInstance;
   /// The consumer of collected dependency information.
   DependencyConsumer &Consumer;
+  /// Callbacks for computing dependency information.
+  DependencyActions &Actions;
   /// Path to the main source file.
   std::string MainFile;
   /// Hash identifying the compilation conditions of the current TU.
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -37,6 +37,8 @@
   std::vector<std::string> Arguments;
 };
 
+/// Dependency scanner callbacks to report dependencies and command-line
+/// invocations.
 class DependencyConsumer {
 public:
   virtual ~DependencyConsumer() {}
@@ -57,6 +59,13 @@
   virtual void handleModuleDependency(ModuleDeps MD) = 0;
 
   virtual void handleContextHash(std::string Hash) = 0;
+};
+
+/// Dependency scanner callbacks that are used during scanning to influence the
+/// behaviour of the scan - for example, to customize the scanned invocations.
+class DependencyActions {
+public:
+  virtual ~DependencyActions();
 
   virtual std::string lookupModuleOutput(const ModuleID &ID,
                                          ModuleOutputKind Kind) = 0;
@@ -83,6 +92,7 @@
   bool computeDependencies(StringRef WorkingDirectory,
                            const std::vector<std::string> &CommandLine,
                            DependencyConsumer &DepConsumer,
+                           DependencyActions &Actions,
                            DiagnosticConsumer &DiagConsumer,
                            std::optional<StringRef> ModuleName = std::nullopt);
   /// \returns A \c StringError with the diagnostic output if clang errors
@@ -90,7 +100,7 @@
   llvm::Error
   computeDependencies(StringRef WorkingDirectory,
                       const std::vector<std::string> &CommandLine,
-                      DependencyConsumer &Consumer,
+                      DependencyConsumer &Consumer, DependencyActions &Actions,
                       std::optional<StringRef> ModuleName = std::nullopt);
 
   bool shouldEagerLoadModules() const { return EagerLoadModules; }
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -143,9 +143,8 @@
 
 class FullDependencyConsumer : public DependencyConsumer {
 public:
-  FullDependencyConsumer(const llvm::StringSet<> &AlreadySeen,
-                         LookupModuleOutputCallback LookupModuleOutput)
-      : AlreadySeen(AlreadySeen), LookupModuleOutput(LookupModuleOutput) {}
+  FullDependencyConsumer(const llvm::StringSet<> &AlreadySeen)
+      : AlreadySeen(AlreadySeen) {}
 
   void handleBuildCommand(Command Cmd) override {
     Commands.push_back(std::move(Cmd));
@@ -169,11 +168,6 @@
     ContextHash = std::move(Hash);
   }
 
-  std::string lookupModuleOutput(const ModuleID &ID,
-                                 ModuleOutputKind Kind) override {
-    return LookupModuleOutput(ID, Kind);
-  }
-
   TranslationUnitDeps takeTranslationUnitDeps();
   ModuleDepsGraph takeModuleGraphDeps();
 
@@ -186,6 +180,22 @@
   std::string ContextHash;
   std::vector<std::string> OutputPaths;
   const llvm::StringSet<> &AlreadySeen;
+};
+
+class CallbackDependencyActions : public DependencyActions {
+public:
+  ~CallbackDependencyActions();
+  CallbackDependencyActions(LookupModuleOutputCallback LookupModuleOutput)
+      : LookupModuleOutput(std::move(LookupModuleOutput)) {
+    assert(this->LookupModuleOutput && "expected LookupModuleOutput");
+  }
+
+  std::string lookupModuleOutput(const ModuleID &ID,
+                                 ModuleOutputKind Kind) override {
+    return LookupModuleOutput(ID, Kind);
+  }
+
+private:
   LookupModuleOutputCallback LookupModuleOutput;
 };
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to