[clang][deps] NFC: Split out the module-based API from the TU-based API

For users of the C++ API, the return type of `getFullDependencies` doesn't make sense when asking for dependencies of a module. In the returned `FullDependenciesResult` instance, only `DiscoveredModules` is useful (the graph of modular dependecies). The `FullDeps` member is trying to describe a translation unit it was never given. Its command line also refers to a file in the in-memory VFS we create in the scanner, leaking the implementation detail.

This patch splits the API and improves layering and naming of the return types.

Depends on D140175.

Reviewed By: artemcm

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

Author: jansvoboda11

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

@@ -28,8 +28,14 @@ namespace dependencies {
 using LookupModuleOutputCallback =
     llvm::function_ref<std::string(const ModuleID &, ModuleOutputKind)>;
 
+/// Graph of modular dependencies.
+using ModuleDepsGraph = std::vector<ModuleDeps>;
+
 /// The full dependencies and module graph for a specific input.
-struct FullDependencies {
+struct TranslationUnitDeps {
+  /// The graph of direct and transitive modular dependencies.
+  ModuleDepsGraph ModuleGraph;
+
   /// The identifier of the C++20 module this translation unit exports.
   ///
   /// If the translation unit is not a module then \c ID.ModuleName is empty.
@@ -62,11 +68,6 @@ struct FullDependencies {
   std::vector<std::string> DriverCommandLine;
 };
 
-struct FullDependenciesResult {
-  FullDependencies FullDeps;
-  std::vector<ModuleDeps> DiscoveredModules;
-};
-
 /// The high-level implementation of the dependency discovery tool that runs on
 /// an individual worker thread.
 class DependencyScanningTool {
@@ -78,18 +79,15 @@ class DependencyScanningTool {
 
   /// Print out the dependency information into a string using the dependency
   /// file format that is specified in the options (-MD is the default) and
-  /// return it. If \p ModuleName isn't empty, this function returns the
-  /// dependency information of module \p ModuleName.
+  /// return it.
   ///
   /// \returns A \c StringError with the diagnostic output if clang errors
   /// occurred, dependency file contents otherwise.
   llvm::Expected<std::string>
-  getDependencyFile(const std::vector<std::string> &CommandLine, StringRef CWD,
-                    std::optional<StringRef> ModuleName = std::nullopt);
+  getDependencyFile(const std::vector<std::string> &CommandLine, StringRef CWD);
 
-  /// Collect the full module dependency graph for the input, ignoring any
-  /// modules which have already been seen. If \p ModuleName isn't empty, this
-  /// function returns the full dependency information of module \p ModuleName.
+  /// Given a Clang driver command-line for a translation unit, gather the
+  /// modular dependencies and return the information needed for explicit build.
   ///
   /// \param AlreadySeen This stores modules which have previously been
   ///                    reported. Use the same instance for all calls to this
@@ -101,12 +99,21 @@ class DependencyScanningTool {
   ///                           arguments for dependencies.
   ///
   /// \returns a \c StringError with the diagnostic output if clang errors
-  /// occurred, \c FullDependencies otherwise.
-  llvm::Expected<FullDependenciesResult>
-  getFullDependencies(const std::vector<std::string> &CommandLine,
-                      StringRef CWD, const llvm::StringSet<> &AlreadySeen,
-                      LookupModuleOutputCallback LookupModuleOutput,
-                      std::optional<StringRef> ModuleName = std::nullopt);
+  /// occurred, \c TranslationUnitDeps otherwise.
+  llvm::Expected<TranslationUnitDeps>
+  getTranslationUnitDependencies(const std::vector<std::string> &CommandLine,
+                                 StringRef CWD,
+                                 const llvm::StringSet<> &AlreadySeen,
+                                 LookupModuleOutputCallback LookupModuleOutput);
+
+  /// Given a compilation context specified via the Clang driver command-line,
+  /// gather modular dependencies of module with the given name, and return the
+  /// information needed for explicit build.
+  llvm::Expected<ModuleDepsGraph>
+  getModuleDependencies(StringRef ModuleName,
+                        const std::vector<std::string> &CommandLine,
+                        StringRef CWD, const llvm::StringSet<> &AlreadySeen,
+                        LookupModuleOutputCallback LookupModuleOutput);
 
 private:
   DependencyScanningWorker Worker;
@@ -145,7 +152,8 @@ class FullDependencyConsumer : public DependencyConsumer {
     return LookupModuleOutput(ID, Kind);
   }
 
-  FullDependenciesResult takeFullDependencies();
+  TranslationUnitDeps takeTranslationUnitDeps();
+  ModuleDepsGraph takeModuleGraphDeps();
 
 private:
   std::vector<std::string> Dependencies;

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

@@ -31,7 +31,7 @@ class DependencyScanningWorkerFilesystem;
 
 /// A command-line tool invocation that is part of building a TU.
 ///
-/// \see FullDependencies::Commands.
+/// \see TranslationUnitDeps::Commands.
 struct Command {
   std::string Executable;
   std::vector<std::string> Arguments;

clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

@@ -20,8 +20,7 @@ DependencyScanningTool::DependencyScanningTool(
     : Worker(Service, std::move(FS)) {}
 
 llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
-    const std::vector<std::string> &CommandLine, StringRef CWD,
-    std::optional<StringRef> ModuleName) {
+    const std::vector<std::string> &CommandLine, StringRef CWD) {
   /// Prints out all of the gathered dependencies into a string.
   class MakeDependencyPrinterConsumer : public DependencyConsumer {
   public:
@@ -81,48 +80,71 @@ llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
   };
 
   MakeDependencyPrinterConsumer Consumer;
-  auto Result =
-      Worker.computeDependencies(CWD, CommandLine, Consumer, ModuleName);
+  auto Result = Worker.computeDependencies(CWD, CommandLine, Consumer);
   if (Result)
     return std::move(Result);
   std::string Output;
   Consumer.printDependencies(Output);
   return Output;
 }
 
-llvm::Expected<FullDependenciesResult>
-DependencyScanningTool::getFullDependencies(
+llvm::Expected<TranslationUnitDeps>
+DependencyScanningTool::getTranslationUnitDependencies(
     const std::vector<std::string> &CommandLine, StringRef CWD,
     const llvm::StringSet<> &AlreadySeen,
-    LookupModuleOutputCallback LookupModuleOutput,
-    std::optional<StringRef> ModuleName) {
+    LookupModuleOutputCallback LookupModuleOutput) {
+  FullDependencyConsumer Consumer(AlreadySeen, LookupModuleOutput);
+  llvm::Error Result = Worker.computeDependencies(CWD, CommandLine, Consumer);
+  if (Result)
+    return std::move(Result);
+  return Consumer.takeTranslationUnitDeps();
+}
+
+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);
   if (Result)
     return std::move(Result);
-  return Consumer.takeFullDependencies();
+  return Consumer.takeModuleGraphDeps();
 }
 
-FullDependenciesResult FullDependencyConsumer::takeFullDependencies() {
-  FullDependenciesResult FDR;
-  FullDependencies &FD = FDR.FullDeps;
+TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() {
+  TranslationUnitDeps TU;
 
-  FD.ID.ContextHash = std::move(ContextHash);
-  FD.FileDeps = std::move(Dependencies);
-  FD.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps);
-  FD.Commands = std::move(Commands);
+  TU.ID.ContextHash = std::move(ContextHash);
+  TU.FileDeps = std::move(Dependencies);
+  TU.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps);
+  TU.Commands = std::move(Commands);
 
   for (auto &&M : ClangModuleDeps) {
     auto &MD = M.second;
     if (MD.ImportedByMainFile)
-      FD.ClangModuleDeps.push_back(MD.ID);
+      TU.ClangModuleDeps.push_back(MD.ID);
+    // TODO: Avoid handleModuleDependency even being called for modules
+    //   we've already seen.
+    if (AlreadySeen.count(M.first))
+      continue;
+    TU.ModuleGraph.push_back(std::move(MD));
+  }
+
+  return TU;
+}
+
+ModuleDepsGraph FullDependencyConsumer::takeModuleGraphDeps() {
+  ModuleDepsGraph ModuleGraph;
+
+  for (auto &&M : ClangModuleDeps) {
+    auto &MD = M.second;
     // TODO: Avoid handleModuleDependency even being called for modules
     //   we've already seen.
     if (AlreadySeen.count(M.first))
       continue;
-    FDR.DiscoveredModules.push_back(std::move(MD));
+    ModuleGraph.push_back(std::move(MD));
   }
 
-  return FDR;
+  return ModuleGraph;
 }

clang/test/ClangScanDeps/modules-full-by-mod-name.cpp

@@ -75,3 +75,5 @@
 // CHECK-NEXT:       "name": "header2"
 // CHECK-NEXT:     }
 // CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": []
+// CHECK-NEXT: }

clang/tools/clang-scan-deps/ClangScanDeps.cpp

@@ -250,29 +250,29 @@ static llvm::json::Array toJSONSorted(std::vector<ModuleID> V) {
 // Thread safe.
 class FullDeps {
 public:
-  void mergeDeps(StringRef Input, FullDependenciesResult FDR,
+  void mergeDeps(StringRef Input, TranslationUnitDeps TUDeps,
                  size_t InputIndex) {
-    FullDependencies &FD = FDR.FullDeps;
-
     InputDeps ID;
     ID.FileName = std::string(Input);
-    ID.ContextHash = std::move(FD.ID.ContextHash);
-    ID.FileDeps = std::move(FD.FileDeps);
-    ID.ModuleDeps = std::move(FD.ClangModuleDeps);
+    ID.ContextHash = std::move(TUDeps.ID.ContextHash);
+    ID.FileDeps = std::move(TUDeps.FileDeps);
+    ID.ModuleDeps = std::move(TUDeps.ClangModuleDeps);
+    mergeDeps(std::move(TUDeps.ModuleGraph), InputIndex);
+    ID.DriverCommandLine = std::move(TUDeps.DriverCommandLine);
+    ID.Commands = std::move(TUDeps.Commands);
+    Inputs.push_back(std::move(ID));
+  }
 
+  void mergeDeps(ModuleDepsGraph Graph, size_t InputIndex) {
     std::unique_lock<std::mutex> ul(Lock);
-    for (const ModuleDeps &MD : FDR.DiscoveredModules) {
+    for (const ModuleDeps &MD : Graph) {
       auto I = Modules.find({MD.ID, 0});
       if (I != Modules.end()) {
         I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
         continue;
       }
       Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
     }
-
-    ID.DriverCommandLine = std::move(FD.DriverCommandLine);
-    ID.Commands = std::move(FD.Commands);
-    Inputs.push_back(std::move(ID));
   }
 
   void printFullOutput(raw_ostream &OS) {
@@ -379,21 +379,38 @@ class FullDeps {
   std::vector<InputDeps> Inputs;
 };
 
-static bool handleFullDependencyToolResult(
-    const std::string &Input,
-    llvm::Expected<FullDependenciesResult> &MaybeFullDeps, FullDeps &FD,
-    size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
-  if (!MaybeFullDeps) {
+static bool handleTranslationUnitResult(
+    StringRef Input, llvm::Expected<TranslationUnitDeps> &MaybeTUDeps,
+    FullDeps &FD, size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
+  if (!MaybeTUDeps) {
     llvm::handleAllErrors(
-        MaybeFullDeps.takeError(), [&Input, &Errs](llvm::StringError &Err) {
+        MaybeTUDeps.takeError(), [&Input, &Errs](llvm::StringError &Err) {
           Errs.applyLocked([&](raw_ostream &OS) {
             OS << "Error while scanning dependencies for " << Input << ":\n";
             OS << Err.getMessage();
           });
         });
     return true;
   }
-  FD.mergeDeps(Input, std::move(*MaybeFullDeps), InputIndex);
+  FD.mergeDeps(Input, std::move(*MaybeTUDeps), InputIndex);
+  return false;
+}
+
+static bool handleModuleResult(
+    StringRef ModuleName, llvm::Expected<ModuleDepsGraph> &MaybeModuleGraph,
+    FullDeps &FD, size_t InputIndex, SharedStream &OS, SharedStream &Errs) {
+  if (!MaybeModuleGraph) {
+    llvm::handleAllErrors(MaybeModuleGraph.takeError(),
+                          [&ModuleName, &Errs](llvm::StringError &Err) {
+                            Errs.applyLocked([&](raw_ostream &OS) {
+                              OS << "Error while scanning dependencies for "
+                                 << ModuleName << ":\n";
+                              OS << Err.getMessage();
+                            });
+                          });
+    return true;
+  }
+  FD.mergeDeps(std::move(*MaybeModuleGraph), InputIndex);
   return false;
 }
 
@@ -571,17 +588,23 @@ int main(int argc, const char **argv) {
 
         // Run the tool on it.
         if (Format == ScanningOutputFormat::Make) {
-          auto MaybeFile = WorkerTools[I]->getDependencyFile(
-              Input->CommandLine, CWD, MaybeModuleName);
+          auto MaybeFile =
+              WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
           if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
                                              Errs))
             HadErrors = true;
+        } else if (MaybeModuleName) {
+          auto MaybeModuleDepsGraph = WorkerTools[I]->getModuleDependencies(
+              *MaybeModuleName, Input->CommandLine, CWD, AlreadySeenModules,
+              LookupOutput);
+          if (handleModuleResult(*MaybeModuleName, MaybeModuleDepsGraph, FD,
+                                 LocalIndex, DependencyOS, Errs))
+            HadErrors = true;
         } else {
-          auto MaybeFullDeps = WorkerTools[I]->getFullDependencies(
-              Input->CommandLine, CWD, AlreadySeenModules, LookupOutput,
-              MaybeModuleName);
-          if (handleFullDependencyToolResult(Filename, MaybeFullDeps, FD,
-                                             LocalIndex, DependencyOS, Errs))
+          auto MaybeTUDeps = WorkerTools[I]->getTranslationUnitDependencies(
+              Input->CommandLine, CWD, AlreadySeenModules, LookupOutput);
+          if (handleTranslationUnitResult(Filename, MaybeTUDeps, FD, LocalIndex,
+                                          DependencyOS, Errs))
             HadErrors = true;
         }
       }