sammccall marked 3 inline comments as done. sammccall added a comment. Sorry about committing this too early, had it confused with another patch. Fixed in 4f000824222f97c0cfd5b19951a1068132e57e79 <https://reviews.llvm.org/rG4f000824222f97c0cfd5b19951a1068132e57e79>
================ Comment at: clang-tools-extra/clangd/TUScheduler.cpp:412 std::unique_ptr<CompilerInvocation> Invocation = buildCompilerInvocation(Inputs, CompilerInvocationDiagConsumer); + // Log cc1 args even (especially!) if creating invocation failed. ---------------- kbobyrev wrote: > `buildCompilerInvocation(Inputs, CompilerInvocationDiagConsumer, &CC1Args)`? > Otherwise, I think the vector would always be empty, right? You're right, I changed this after testing it. Fixed in 4f000824222f97c0cfd5b19951a1068132e57e79 ================ Comment at: clang/include/clang/Frontend/Utils.h:231 + bool ShouldRecoverOnErrors = false, + std::vector<std::string> *CC1Args = nullptr); ---------------- kbobyrev wrote: > Nit: `llvm::Optional` might look better for `CC1Args` Doesn't really work as we want optional + pass-by-reference here - a pointer is the usual idiom. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70832/new/ https://reviews.llvm.org/D70832 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits