This is an archive of the discontinued LLVM Phabricator instance.

[clangd] NFC: Use buildCompilerInvocation in CodeComplete
ClosedPublic

Authored by kadircet on Jan 17 2019, 8:50 AM.

Diff Detail

Event Timeline

kadircet created this revision.Jan 17 2019, 8:50 AM
ilya-biryukov added inline comments.Jan 21 2019, 8:01 AM
clangd/CodeComplete.cpp
1028–1029

remove this line, buildCompilerInvocation handles this for us

1029–1030

remove this line, buildCompilerInvocation handles this for us

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.

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.
The function was made public in 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.

kadircet updated this revision to Diff 182856.Jan 22 2019, 1:27 AM
kadircet marked 2 inline comments as done.
  • Moved ParseInputs and buildCompilerInvocation into Compiler.h
sammccall accepted this revision.Jan 22 2019, 1:41 AM

Thank you!

This revision is now accepted and ready to land.Jan 22 2019, 1:41 AM
This revision was automatically updated to reflect the committed changes.