jansvoboda11 accepted this revision.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.
LGTM overall with some minor nitpicks.
================
Comment at:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:95
+ const llvm::StringSet<> &AlreadySeen,
+ llvm::function_ref<std::string(const ModuleID &, ModuleOutputKind)>
+ LookupModuleOutput,
----------------
Creating a type alias might make this more readable & easier to refactor later.
================
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:102
+ if (!CI.getDiagnosticOpts().DiagnosticSerializationFile.empty())
+ CI.getDiagnosticOpts().DiagnosticSerializationFile = "-";
+ if (!CI.getDependencyOutputOpts().OutputFile.empty())
----------------
Can you point out in a comment that this value is just temporary for context
hash computation and will be replaced by real path later on?
================
Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:403
+
+static std::string getModuleCachePath(ArrayRef<std::string> Args) {
+ for (StringRef Arg : llvm::reverse(Args)) {
----------------
Can you split this into separate patch?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131934/new/
https://reviews.llvm.org/D131934
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
