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

Reply via email to