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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
