[clang] 296ba5b - [clang][deps] Split lookupModuleOutput out of DependencyConsumer NFC

Ben Langmuir via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 10 13:15:06 PST 2023


Author: Ben Langmuir
Date: 2023-03-10T13:14:49-08:00
New Revision: 296ba5bbd387e7f3edb0131054183fa7913a091e

URL: https://github.com/llvm/llvm-project/commit/296ba5bbd387e7f3edb0131054183fa7913a091e
DIFF: https://github.com/llvm/llvm-project/commit/296ba5bbd387e7f3edb0131054183fa7913a091e.diff

LOG: [clang][deps] Split lookupModuleOutput out of DependencyConsumer NFC

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 (DependencyActionController). Currently this
is just lookupModuleOutput, but we have additional callbacks related to
CAS support that we intend to upstream in the future.

Differential Revision: https://reviews.llvm.org/D144058

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 13d42867bdd58..87a4299c7f1b9 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -143,9 +143,8 @@ class DependencyScanningTool {
 
 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 @@ class FullDependencyConsumer : public DependencyConsumer {
     ContextHash = std::move(Hash);
   }
 
-  std::string lookupModuleOutput(const ModuleID &ID,
-                                 ModuleOutputKind Kind) override {
-    return LookupModuleOutput(ID, Kind);
-  }
-
   TranslationUnitDeps takeTranslationUnitDeps();
   ModuleDepsGraph takeModuleGraphDeps();
 
@@ -186,6 +180,30 @@ class FullDependencyConsumer : public DependencyConsumer {
   std::string ContextHash;
   std::vector<std::string> OutputPaths;
   const llvm::StringSet<> &AlreadySeen;
+};
+
+/// A simple dependency action controller that uses a callback. If no callback
+/// is provided, it is assumed that looking up module outputs is unreachable.
+class CallbackActionController : public DependencyActionController {
+public:
+  virtual ~CallbackActionController();
+
+  CallbackActionController(LookupModuleOutputCallback LMO)
+      : LookupModuleOutput(std::move(LMO)) {
+    if (!LookupModuleOutput) {
+      LookupModuleOutput = [](const ModuleID &,
+                              ModuleOutputKind) -> std::string {
+        llvm::report_fatal_error("unexpected call to lookupModuleOutput");
+      };
+    }
+  }
+
+  std::string lookupModuleOutput(const ModuleID &ID,
+                                 ModuleOutputKind Kind) override {
+    return LookupModuleOutput(ID, Kind);
+  }
+
+private:
   LookupModuleOutputCallback LookupModuleOutput;
 };
 

diff  --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 5ada65c0a8308..350acb8f8a79e 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -57,6 +57,13 @@ class DependencyConsumer {
   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 DependencyActionController {
+public:
+  virtual ~DependencyActionController();
 
   virtual std::string lookupModuleOutput(const ModuleID &ID,
                                          ModuleOutputKind Kind) = 0;
@@ -83,15 +90,15 @@ class DependencyScanningWorker {
   bool computeDependencies(StringRef WorkingDirectory,
                            const std::vector<std::string> &CommandLine,
                            DependencyConsumer &DepConsumer,
+                           DependencyActionController &Controller,
                            DiagnosticConsumer &DiagConsumer,
                            std::optional<StringRef> ModuleName = std::nullopt);
   /// \returns A \c StringError with the diagnostic output if clang errors
   /// occurred, success otherwise.
-  llvm::Error
-  computeDependencies(StringRef WorkingDirectory,
-                      const std::vector<std::string> &CommandLine,
-                      DependencyConsumer &Consumer,
-                      std::optional<StringRef> ModuleName = std::nullopt);
+  llvm::Error computeDependencies(
+      StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
+      DependencyConsumer &Consumer, DependencyActionController &Controller,
+      std::optional<StringRef> ModuleName = std::nullopt);
 
   bool shouldEagerLoadModules() const { return EagerLoadModules; }
 

diff  --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 238fc84ddd11c..24d7337d26e4c 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -27,6 +27,7 @@ namespace clang {
 namespace tooling {
 namespace dependencies {
 
+class DependencyActionController;
 class DependencyConsumer;
 
 /// Modular dependency that has already been built prior to the dependency scan.
@@ -201,6 +202,7 @@ class ModuleDepCollector final : public DependencyCollector {
 public:
   ModuleDepCollector(std::unique_ptr<DependencyOutputOptions> Opts,
                      CompilerInstance &ScanInstance, DependencyConsumer &C,
+                     DependencyActionController &Controller,
                      CompilerInvocation OriginalCI, bool OptimizeArgs,
                      bool EagerLoadModules, bool IsStdModuleP1689Format);
 
@@ -218,6 +220,8 @@ class ModuleDepCollector final : public DependencyCollector {
   CompilerInstance &ScanInstance;
   /// The consumer of collected dependency information.
   DependencyConsumer &Consumer;
+  /// Callbacks for computing dependency information.
+  DependencyActionController &Controller;
   /// Path to the main source file.
   std::string MainFile;
   /// Hash identifying the compilation conditions of the current TU.

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index c15f6e724fa14..6641518b572b1 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -46,11 +46,6 @@ class MakeDependencyPrinterConsumer : public DependencyConsumer {
 
   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.");
 
@@ -82,7 +77,9 @@ class MakeDependencyPrinterConsumer : public DependencyConsumer {
 llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
     const std::vector<std::string> &CommandLine, StringRef CWD) {
   MakeDependencyPrinterConsumer Consumer;
-  auto Result = Worker.computeDependencies(CWD, CommandLine, Consumer);
+  CallbackActionController Controller(nullptr);
+  auto Result =
+      Worker.computeDependencies(CWD, CommandLine, Consumer, Controller);
   if (Result)
     return std::move(Result);
   std::string Output;
@@ -117,20 +114,25 @@ llvm::Expected<P1689Rule> DependencyScanningTool::getP1689ModuleDependencyFile(
       return Opts->OutputFile;
     }
 
-    // The lookupModuleOutput is for clang modules. P1689 format don't need it.
-    std::string lookupModuleOutput(const ModuleID &ID,
-                                 ModuleOutputKind Kind) override {
-      return "";
-    }
-
   private:
     StringRef Filename;
     P1689Rule &Rule;
   };
 
+  class P1689ActionController : public DependencyActionController {
+  public:
+    // The lookupModuleOutput is for clang modules. P1689 format don't need it.
+    std::string lookupModuleOutput(const ModuleID &,
+                                   ModuleOutputKind Kind) override {
+      return "";
+    }
+  };
+
   P1689Rule Rule;
   P1689ModuleDependencyPrinterConsumer Consumer(Rule, Command);
-  auto Result = Worker.computeDependencies(CWD, Command.CommandLine, Consumer);
+  P1689ActionController Controller;
+  auto Result = Worker.computeDependencies(CWD, Command.CommandLine, Consumer,
+                                           Controller);
   if (Result)
     return std::move(Result);
 
@@ -145,8 +147,10 @@ DependencyScanningTool::getTranslationUnitDependencies(
     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);
+  CallbackActionController Controller(LookupModuleOutput);
+  llvm::Error Result =
+      Worker.computeDependencies(CWD, CommandLine, Consumer, Controller);
   if (Result)
     return std::move(Result);
   return Consumer.takeTranslationUnitDeps();
@@ -156,9 +160,10 @@ llvm::Expected<ModuleDepsGraph> DependencyScanningTool::getModuleDependencies(
     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);
+  CallbackActionController Controller(LookupModuleOutput);
+  llvm::Error Result = Worker.computeDependencies(CWD, CommandLine, Consumer,
+                                                  Controller, ModuleName);
   if (Result)
     return std::move(Result);
   return Consumer.takeModuleGraphDeps();
@@ -200,3 +205,5 @@ ModuleDepsGraph FullDependencyConsumer::takeModuleGraphDeps() {
 
   return ModuleGraph;
 }
+
+CallbackActionController::~CallbackActionController() {}

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index f7c976b4a3033..10a26ab05ddf9 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -146,13 +146,14 @@ class DependencyScanningAction : public tooling::ToolAction {
 public:
   DependencyScanningAction(
       StringRef WorkingDirectory, DependencyConsumer &Consumer,
+      DependencyActionController &Controller,
       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) {}
+        Controller(Controller), DepFS(std::move(DepFS)), Format(Format),
+        OptimizeArgs(OptimizeArgs), EagerLoadModules(EagerLoadModules),
+        DisableFree(DisableFree), ModuleName(ModuleName) {}
 
   bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
                      FileManager *FileMgr,
@@ -250,8 +251,8 @@ class DependencyScanningAction : public tooling::ToolAction {
     case ScanningOutputFormat::P1689:
     case ScanningOutputFormat::Full:
       MDC = std::make_shared<ModuleDepCollector>(
-          std::move(Opts), ScanInstance, Consumer, OriginalInvocation,
-          OptimizeArgs, EagerLoadModules,
+          std::move(Opts), ScanInstance, Consumer, Controller,
+          OriginalInvocation, OptimizeArgs, EagerLoadModules,
           Format == ScanningOutputFormat::P1689);
       ScanInstance.addDependencyCollector(MDC);
       break;
@@ -300,6 +301,7 @@ class DependencyScanningAction : public tooling::ToolAction {
 private:
   StringRef WorkingDirectory;
   DependencyConsumer &Consumer;
+  DependencyActionController &Controller;
   llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
   ScanningOutputFormat Format;
   bool OptimizeArgs;
@@ -342,7 +344,8 @@ DependencyScanningWorker::DependencyScanningWorker(
 
 llvm::Error DependencyScanningWorker::computeDependencies(
     StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
-    DependencyConsumer &Consumer, std::optional<StringRef> ModuleName) {
+    DependencyConsumer &Consumer, DependencyActionController &Controller,
+    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::Error DependencyScanningWorker::computeDependencies(
   llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput);
   TextDiagnosticPrinter DiagPrinter(DiagnosticsOS, DiagOpts.release());
 
-  if (computeDependencies(WorkingDirectory, CommandLine, Consumer, DiagPrinter,
-                          ModuleName))
+  if (computeDependencies(WorkingDirectory, CommandLine, Consumer, Controller,
+                          DiagPrinter, ModuleName))
     return llvm::Error::success();
   return llvm::make_error<llvm::StringError>(DiagnosticsOS.str(),
                                              llvm::inconvertibleErrorCode());
@@ -388,8 +391,8 @@ static bool forEachDriverJob(
 
 bool DependencyScanningWorker::computeDependencies(
     StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
-    DependencyConsumer &Consumer, DiagnosticConsumer &DC,
-    std::optional<StringRef> ModuleName) {
+    DependencyConsumer &Consumer, DependencyActionController &Controller,
+    DiagnosticConsumer &DC, std::optional<StringRef> ModuleName) {
   // Reset what might have been modified in the previous worker invocation.
   BaseFS->setCurrentWorkingDirectory(WorkingDirectory);
 
@@ -445,9 +448,9 @@ bool DependencyScanningWorker::computeDependencies(
   // 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, Controller, 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 @@ bool DependencyScanningWorker::computeDependencies(
         << llvm::join(FinalCommandLine, " ");
   return Success && Action.hasScanned();
 }
+
+DependencyActionController::~DependencyActionController() {}

diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index e882067dca398..8cac033742a8c 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -56,16 +56,16 @@ static std::vector<std::string> splitString(std::string S, char Separator) {
 void ModuleDepCollector::addOutputPaths(CompilerInvocation &CI,
                                         ModuleDeps &Deps) {
   CI.getFrontendOpts().OutputFile =
-      Consumer.lookupModuleOutput(Deps.ID, ModuleOutputKind::ModuleFile);
+      Controller.lookupModuleOutput(Deps.ID, ModuleOutputKind::ModuleFile);
   if (!CI.getDiagnosticOpts().DiagnosticSerializationFile.empty())
     CI.getDiagnosticOpts().DiagnosticSerializationFile =
-        Consumer.lookupModuleOutput(
+        Controller.lookupModuleOutput(
             Deps.ID, ModuleOutputKind::DiagnosticSerializationFile);
   if (!CI.getDependencyOutputOpts().OutputFile.empty()) {
-    CI.getDependencyOutputOpts().OutputFile =
-        Consumer.lookupModuleOutput(Deps.ID, ModuleOutputKind::DependencyFile);
+    CI.getDependencyOutputOpts().OutputFile = Controller.lookupModuleOutput(
+        Deps.ID, ModuleOutputKind::DependencyFile);
     CI.getDependencyOutputOpts().Targets =
-        splitString(Consumer.lookupModuleOutput(
+        splitString(Controller.lookupModuleOutput(
                         Deps.ID, ModuleOutputKind::DependencyTargets),
                     '\0');
     if (!CI.getDependencyOutputOpts().OutputFile.empty() &&
@@ -205,7 +205,7 @@ void ModuleDepCollector::addModuleFiles(
     CompilerInvocation &CI, ArrayRef<ModuleID> ClangModuleDeps) const {
   for (const ModuleID &MID : ClangModuleDeps) {
     std::string PCMPath =
-        Consumer.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
+        Controller.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
     if (EagerLoadModules)
       CI.getFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
     else
@@ -577,11 +577,11 @@ void ModuleDepCollectorPP::addAffectingClangModule(
 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),
+    DependencyActionController &Controller, CompilerInvocation OriginalCI,
+    bool OptimizeArgs, bool EagerLoadModules, bool IsStdModuleP1689Format)
+    : ScanInstance(ScanInstance), Consumer(C), Controller(Controller),
+      Opts(std::move(Opts)), OriginalInvocation(std::move(OriginalCI)),
+      OptimizeArgs(OptimizeArgs), EagerLoadModules(EagerLoadModules),
       IsStdModuleP1689Format(IsStdModuleP1689Format) {}
 
 void ModuleDepCollector::attachToPreprocessor(Preprocessor &PP) {


        


More information about the cfe-commits mailing list