[PATCH] D56860: [clangd] NFC: Use buildCompilerInvocation in CodeComplete

Mon Jan 21 08:40:31 PST 2019

sammccall added a comment.

In D56860#1365432 <https://reviews.llvm.org/D56860#1365432>, @ilya-biryukov wrote:

> Adding Sam as a reviewer, IIRC we used to have `buildCompilerInvocation` here and he was the one who inlined it.
>  I would personally LGTM this change (with the two comments fixed), but Sam might have a different opinion on whether this should be outlined again.

I inlined it because `buildCompilerInvocation` was private, and I was moving CodeComplete out of ClangdUnit.cpp in rL319655 <https://reviews.llvm.org/rL319655>.
The function was made public in rL333737 <https://reviews.llvm.org/rL333737> as part of a larger change.
I don't really object to the patch as it is, but if you're open to suggestions...

I'd **prefer** to have fewer dependencies between CodeComplete and ClangdUnit, as the latter has historically been a hairball.
Can we move `buildCompilerInvocation` and `ParseInputs` into `Compiler.h`, which is simplified APIs for invoking the compiler?
Both ClangdUnit and CodeComplete can certainly use Compiler, no question.

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56860/new/

https://reviews.llvm.org/D56860