[PATCH] D120713: [clangd] Make dexp command line options sticky

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban closed this revision.
yrouban added a comment.

Closed by commit 
https://reviews.llvm.org/rG7fb39fb6d6665cd469557b43eb205cc32b0a7ac3.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120713

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120713: [clangd] Make dexp command line options sticky

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:418
+  // Protect IndexLocation, ExecCommand and ProjectRoot from being reset.
+  IndexLocation.setValue(IndexLocation, true /* initial */);
+  ExecCommand.setValue(ExecCommand, true /* initial */);

also nit: `/*initial=*/ true`. same below


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120713

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120713: [clangd] Make dexp command line options sticky

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thank you!




Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:417
+
+  // Protect IndexLocation, ExecCommand and ProjectRoot from being reset.
+  IndexLocation.setValue(IndexLocation, true /* initial */);

nit: comment echoes the code, say why instead?

Preserve global options when flag parser is reset, so commands can use them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120713

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120713: [clangd] Make dexp command line options sticky

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban added subscribers: jhenderson, lattner.
yrouban added a comment.

In D120713#3353481 , @lattner wrote:

> I'm not a competent reviewer for clangd
>
>> ! In D120713#3353532 , @jhenderson 
>> wrote:
>
> I don't know anything about clangd either. Not sure why I was added as a 
> reviewer :)

I took your names from https://reviews.llvm.org/D105959 which affected Dexp.cpp 
and was related to cl::opt.
Ok, I will shoot in the dark once more :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120713

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120713: [clangd] Make dexp command line options sticky

[James Henderson via Phabricator via cfe-commits]

jhenderson resigned from this revision.
jhenderson added a comment.

I don't know anything about clangd either. Not sure why I was added as a 
reviewer :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120713

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120713: [clangd] Make dexp command line options sticky

[Chris Lattner via Phabricator via cfe-commits]

lattner resigned from this revision.
lattner added a comment.
Herald added a project: All.

I'm not a competent reviewer for clangd


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120713

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120713: [clangd] Make dexp command line options sticky

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban created this revision.
yrouban added reviewers: jhenderson, lattner, jpienaar, mehdi_amini.
yrouban added a project: clang.
Herald added subscribers: usaxena95, kadircet, arphaman.
yrouban requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Preparing for the cl::opt reset fix proposed on D115433 
 this patch fixes the dexp tool to preserve 
its three command line options (IndexLocation, ExecCommand, ProjectRoot) from 
reset that is done before parsing query options.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120713

Files:
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp


Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -413,6 +413,12 @@
   using namespace clang::clangd;
 
   llvm::cl::ParseCommandLineOptions(argc, argv, Overview);
+
+  // Protect IndexLocation, ExecCommand and ProjectRoot from being reset.
+  IndexLocation.setValue(IndexLocation, true /* initial */);
+  ExecCommand.setValue(ExecCommand, true /* initial */);
+  ProjectRoot.setValue(ProjectRoot, true /* initial */);
+
   llvm::cl::ResetCommandLineParser(); // We reuse it for REPL commands.
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 


Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -413,6 +413,12 @@
   using namespace clang::clangd;
 
   llvm::cl::ParseCommandLineOptions(argc, argv, Overview);
+
+  // Protect IndexLocation, ExecCommand and ProjectRoot from being reset.
+  IndexLocation.setValue(IndexLocation, true /* initial */);
+  ExecCommand.setValue(ExecCommand, true /* initial */);
+  ProjectRoot.setValue(ProjectRoot, true /* initial */);
+
   llvm::cl::ResetCommandLineParser(); // We reuse it for REPL commands.
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits