Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land.
lgtm with the changes to `FindResourceDir`. ================ Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:50 + /// compiler path is NOT an absolute path. + std::string FindResourceDir(const tooling::CommandLineArguments &Args) { + if (Args.size() < 1) ---------------- This doesn't need to return a `std::string` as the lifetime of the returned string is the same as the lifetime of `ResourceDirectoryCache::Cache`. It can be a `StringRef` instead. ================ Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:50 + /// compiler path is NOT an absolute path. + std::string FindResourceDir(const tooling::CommandLineArguments &Args) { + if (Args.size() < 1) ---------------- Bigcheese wrote: > This doesn't need to return a `std::string` as the lifetime of the returned > string is the same as the lifetime of `ResourceDirectoryCache::Cache`. It can > be a `StringRef` instead. `FindResourceDir` should be `findResourceDir`. ================ Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:75-79 + llvm::Optional<StringRef> Redirects[] = { + {""}, // Stdin + StringRef(OutputFile), + StringRef(ErrorFile), + }; ---------------- It would be nice if this didn't need to be a real file, but it looks like `llvm::sys::Execute` can only handle file paths. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69122/new/ https://reviews.llvm.org/D69122 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits