[PATCH] D77645: [clangd] Support dexp -c "some command"
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 8 05:23:21 PDT 2020
sammccall marked 4 inline comments as done.
sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:100
+ // must do this before opts are destroyed
+ auto Cleanup = llvm::make_scope_exit(llvm::cl::ResetCommandLineParser);
if (Help.getNumOccurrences() > 0) {
----------------
kbobyrev wrote:
> nit: const?
we generally don't bother declaring local vars const.
================
Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:328
+ if (!ExecCommand.empty())
+ return runCommand(ExecCommand, *Index) ? 0 : 1;
----------------
kbobyrev wrote:
> `return !runCommand(...)`?
I think it's unneccesarily confusing to rely on implicit bool->int conversion in a context where zero means success.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77645/new/
https://reviews.llvm.org/D77645
More information about the cfe-commits
mailing list