[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese marked 2 inline comments as done.
Bigcheese added inline comments.


================
Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:31
+  /// Modules that the input directly imports.
+  std::vector<std::string> DirectModuleDependencies;
+  /// The Clang modules this input transitively depends on that have not already
----------------
arphaman wrote:
> Are the strings supposed to represent the module names here? For C++20 modules, will a single string be enough to represent both the module's name and its partition name?
If it's a partition the module name will just contain a `:`.


================
Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:63
+  llvm::Expected<FullDependencies>
+  getFullDependencies(const tooling::CompilationDatabase &Compilations,
+                      StringRef CWD, const llvm::StringSet<> &AlreadySeen);
----------------
arphaman wrote:
> Can you add a comment on how `AlreadySeen` is supposed to be used. Are clients expected to update it once they get more dependencies?
Will do. I'm still working out exactly how `AlreadySeen` should work. Ideally it would be shared across all workers of the same `DependencyScanningService`, but that needs thread synchronization and could have rather high overhead. The current model is that it's per worker, and you should expect to need to deduplicate modules across workers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268