[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