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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
