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

[jansvoboda11]

Summary

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.

[artemcm]

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

112

The existing client of the ModuleName API queries a single module and it's nice to get a structured output of a FullDependenciesResult for the module being queried itself, and get its dependencies from that. Whereas now we will just be getting a sequence of ModuleDepss. So just to confirm, if a client queries getModuleDependencies("Foo", ...), the resulting ModuleDepsGraph will contain a ModuleDeps for Foo itself also, and not just Foo's dependencies?

Still, the client will now need to traverse this graph to find Foo in particular, and it'd be nice to not have to do that. TranslationUnitDeps is not the right thing here, but perhaps something simpler could work, along the lines of:

struct SingleModuleDeps {
  ModuleDeps mainQueriedModule

  /// The graph of direct and transitive modular dependencies of mainQueriedModule
  ModuleDepsGraph ModuleGraph;
};

What do you think?

[jansvoboda11]

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

112

Yes, ModuleDepsGraph will contain both the dependencies and Foo itself.

Looking at the client code, it doesn't look like it's making any use of the structured nature of FullDependenciesResult, right? The unused FullDependenciesResult::FullDeps field doesn't hold anything useful ATM and FullDependenciesResult::DiscoveredModules is equivalent to the new ModuleDepsGraph. Your client iterates over the whole (flat) dependency graph, creates a cache (map from the module name to its information), and then looks up the queried module in the map.

I'm not sure the suggested return type is buying you much. You probably still need to iterate over the whole graph to create the associative cache. (I guess you might save one insert and find, since we'd give you the root explicitly.) If knowing information on the queried module enables something more interesting, I propose this:

struct SingleModuleDeps {
  ModuleID RootModuleID;
  std::unordered_map<ModuleID, ModuleDeps> ModuleGraph;
};

This gives you the option to look up information on the root module in constant time, but has the advantage of not "splitting" the dependency graph into two parts (the queried module and the rest). I think this might be useful in the future. Let's say your client learns it needs dependencies of a bunch of Clang modules. It could send them all to the Clang scanner at once, which might have some advantages: better parallelism and returning single dependency graph (less allocations and data copies if parts are shared). If we stored ModuleDeps for the queried modules outside of the rest of the ModuleGraph, it becomes hard to model dependencies between the queried modules.

Let me know what you think. I'm happy to do this, but maybe in a follow up patch?

[artemcm]

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

112

I like your proposed SingleModuleDeps design, and think that it'd be useful to both be able to identify the root in constant time and the potential followup use-cases that could benefit from it sound nice too. Doing it in a followup patch sounds just fine. Thanks!

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

 namespace clang {
 namespace tooling {
 namespace dependencies {
 
 /// A callback to lookup module outputs for "-fmodule-file=", "-o" etc.
 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.
   ModuleID ID;
 
   /// A collection of absolute paths to files that this translation unit
   /// directly depends on, not including transitive dependencies.
   std::vector<std::string> FileDeps;
struct TranslationUnitDeps {
   /// should make the dependencies between commands explicit to enable parallel
   /// builds of each architecture.
   std::vector<Command> Commands;
 
   /// Deprecated driver command-line. This will be removed in a future version.
   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 {
 public:
   /// Construct a dependency scanning tool.
   DependencyScanningTool(DependencyScanningService &Service,
                          llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS =
                              llvm::vfs::createPhysicalFileSystem());
 
   /// 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
   ///                    function for a single \c DependencyScanningTool in a
   ///                    single build. Use a different one for different tools,
   ///                    and clear it between builds.
   /// \param LookupModuleOutput This function is called to fill in
   ///                           "-fmodule-file=", "-o" and other output
   ///                           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,
+  /// 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,
-                      std::optional<StringRef> ModuleName = std::nullopt);
+                        LookupModuleOutputCallback LookupModuleOutput);
 
 private:
   DependencyScanningWorker Worker;
 };
 
 class FullDependencyConsumer : public DependencyConsumer {
 public:
   FullDependencyConsumer(const llvm::StringSet<> &AlreadySeen,
void handleContextHash(std::string Hash) override {
     ContextHash = std::move(Hash);
   }
 
   std::string lookupModuleOutput(const ModuleID &ID,
                                  ModuleOutputKind Kind) override {
     return LookupModuleOutput(ID, Kind);
   }
 
-  FullDependenciesResult takeFullDependencies();
+  TranslationUnitDeps takeTranslationUnitDeps();
+  ModuleDepsGraph takeModuleGraphDeps();
 
 private:
   std::vector<std::string> Dependencies;
   std::vector<PrebuiltModuleDep> PrebuiltModuleDeps;
   llvm::MapVector<std::string, ModuleDeps, llvm::StringMap<unsigned>>
       ClangModuleDeps;
   std::vector<Command> Commands;
   std::string ContextHash;

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

 
 namespace tooling {
 namespace dependencies {
 
 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;
 };
 
 class DependencyConsumer {
 public:
   virtual ~DependencyConsumer() {}

clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

 using namespace dependencies;
 
 DependencyScanningTool::DependencyScanningTool(
     DependencyScanningService &Service,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
     : 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:
     void handleBuildCommand(Command) override {}
 
     void
     handleDependencyOutputOpts(const DependencyOutputOptions &Opts) override {
       this->Opts = std::make_unique<DependencyOutputOptions>(Opts);
public:
     }
 
   private:
     std::unique_ptr<DependencyOutputOptions> Opts;
     std::vector<std::string> Dependencies;
   };
 
   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

 // CHECK-NEXT:       "context-hash": "[[HASH_H2_DINCLUDE]]",
 // CHECK-NEXT:       "file-deps": [
 // CHECK-NEXT:         "[[PREFIX]]/Inputs/header2.h",
 // CHECK-NEXT:         "[[PREFIX]]/Inputs/module.modulemap"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "name": "header2"
 // CHECK-NEXT:     }
 // CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": []
+// CHECK-NEXT: }

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

for (const ModuleID &MID : V)
     Ret.push_back(llvm::json::Object(
         {{"module-name", MID.ModuleName}, {"context-hash", MID.ContextHash}}));
   return Ret;
 }
 
 // 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) {
     // Sort the modules by name to get a deterministic order.
     std::vector<IndexedModuleID> ModuleIDs;
     for (auto &&M : Modules)
       ModuleIDs.push_back(M.first);
     llvm::sort(ModuleIDs,
private:
   };
 
   std::mutex Lock;
   std::unordered_map<IndexedModuleID, ModuleDeps, IndexedModuleIDHasher>
       Modules;
   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;
 }
 
 /// Construct a path for the explicitly built PCM.
 static std::string constructPCMPath(ModuleID MID, StringRef OutputDir) {
   SmallString<256> ExplicitPCMPath(OutputDir);
   llvm::sys::path::append(ExplicitPCMPath, MID.ContextHash,
                           MID.ModuleName + "-" + MID.ContextHash + ".pcm");
Pool.async([I, &Lock, &Index, &Inputs, &HadErrors, &FD, &WorkerTools,
         if (OutputDir.empty())
           OutputDir = getModuleCachePath(Input->CommandLine);
         auto LookupOutput = [&](const ModuleID &MID, ModuleOutputKind MOK) {
           return ::lookupModuleOutput(MID, MOK, OutputDir);
         };
 
         // 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 {
-          auto MaybeFullDeps = WorkerTools[I]->getFullDependencies(
-              Input->CommandLine, CWD, AlreadySeenModules, LookupOutput,
-              MaybeModuleName);
-          if (handleFullDependencyToolResult(Filename, MaybeFullDeps, FD,
-                                             LocalIndex, DependencyOS, Errs))
+        } else if (MaybeModuleName) {
+          auto MaybeModuleDepsGraph = WorkerTools[I]->getModuleDependencies(
+              *MaybeModuleName, Input->CommandLine, CWD, AlreadySeenModules,
+              LookupOutput);
+          if (handleModuleResult(*MaybeModuleName, MaybeModuleDepsGraph, FD,
             HadErrors = true;
+        } else {
+          auto MaybeTUDeps = WorkerTools[I]->getTranslationUnitDependencies(
+              Input->CommandLine, CWD, AlreadySeenModules, LookupOutput);
+          if (handleTranslationUnitResult(Filename, MaybeTUDeps, FD, LocalIndex,
+                                          DependencyOS, Errs))
+            HadErrors = true;
         }
       }
     });
   }
   Pool.wait();
 
   if (Format == ScanningOutputFormat::Full)
     FD.printFullOutput(llvm::outs());
 
   return HadErrors;
 }