Re: [Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

Thanks, Zachary also pinged me on IRC. This was fixed in r347821. > On Nov 28, 2018, at 4:09 PM, Leonard Mosescu via Phabricator > wrote: > > lemo added a comment. > > I noticed a small problem, this change breaks "lldb -c ". The > inline comment explains the root cause. > > Thanks > > >

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

lemo added a comment. I noticed a small problem, this change breaks "lldb -c ". The inline comment explains the root cause. Thanks Comment at: lldb/trunk/tools/driver/Driver.cpp:310 + if (args.hasArg(OPT_core)) { +SBFileSpec file(optarg); +if (file.Exists()) { --

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

This revision was automatically updated to reflect the committed changes. Closed by commit rL347709: [Driver] Use libOption with tablegen. (authored by JDevlieghere, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D54692?vs=175526&id=17

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere updated this revision to Diff 175526. JDevlieghere added a comment. - Add EXAMPLES section CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 Files: lit/Driver/Inputs/Print0.in lit/Driver/Inputs/Print2.in lit/Driver/Inputs/Print4

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

clayborg added a comment. I would be happy if we just have an EXAMPLES section much like many of the man pages for built in shell commands where we show a few examples. I would like to see setting up a target a program with arguments that might conflict with the argument parser like: % lldb

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere updated this revision to Diff 175505. JDevlieghere edited the summary of this revision. JDevlieghere added a comment. - Add lit test for command options. - Change usage with prose. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 Fil

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

labath added inline comments. Comment at: tools/driver/Driver.cpp:876-888 + usage << indent << tool_name +<< " -a -f [-c ] [-s ] [-o " + "] [-S ] [-O ] [-k ] [-K ] " + "[-Q] [-b] [-e] [-x] [-X] [-l ] [-d] [-z " + "] [[--] [ ...]]\n"; + u

Re: [Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

One potentially nice way to test this would be to have all the -O, -o, -S, -s, -Q, and -q just run "script print(N)" where N is some number that changes at each line, then FileCheck the output On Mon, Nov 26, 2018 at 6:38 PM Jim Ingham via Phabricator < revi...@reviews.llvm.org> wrote: > jingham

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

jingham added a comment. You have to gather all the -O's and -S's and -Q's and add them to the list of code that gets sourced before the file is loaded in the order in which you find them. There can be more than one of each of these and they can be interspersed anywhere among the other command

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere updated this revision to Diff 175377. JDevlieghere added a comment. Will land this tomorrow once we've figured out how to integrate tablegen with the Xcode project. In the meantime I found some issues due to ordering of command options. Their relative order matters and we had tests

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere added a comment. In D54692#1308640 , @clayborg wrote: > I would wait to get another OK from anyone else just to be sure. Sounds good. Thanks Greg! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

clayborg added a comment. I would wait to get another OK from anyone else just to be sure. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 ___ lldb-commits mailing list lldb-commits@lists.llvm.org ht

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere added a comment. In D54692#1308604 , @clayborg wrote: > Can you attach new output with the grouping and extra usage? I updated the original snippet, see https://reviews.llvm.org/P8118 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

clayborg added a comment. Can you attach new output with the grouping and extra usage? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http:/

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere updated this revision to Diff 175319. JDevlieghere added a comment. - Add grouping - Use basename for help CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 Files: tools/driver/CMakeLists.txt tools/driver/Driver.cpp tools/driver

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

jingham added a comment. I didn't read the patch in detail yet, these are just meta-comments: It looks like the libOption approach (or this implementation of it) is missing the notion of option groups. That's what you see in the first section of the lldb --help printout in the current version.

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

clayborg added a comment. I know cl::opt had ways to group options and the table gen is more powerful so it must have this feature? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 ___ lldb-commits m

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

clayborg added a comment. Can we still group the options as mentioned in my previous comment? Comment at: tools/driver/Driver.cpp:943 +usage << '\n'; +usage << argv[0] << " -h" << '\n'; +usage << argv[0] << " -v [[--] [ ...]]\n"; Get file base name

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere updated this revision to Diff 175300. JDevlieghere added a comment. Add old usage. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 Files: tools/driver/CMakeLists.txt tools/driver/Driver.cpp tools/driver/Driver.h tools/driver

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

clayborg added a comment. the old usage: Usage: lldb -h lldb -v [[--] [ ...]] lldb -a -f [-c ] [-s ] [-o ] [-S ] [-O ] [-k ] [-K ] [-Q] [-b] [-e] [-x] [-X] [-l ] [-d] [-z ] [[--] [ ...]] lldb -n -w [-s ] [-o ] [-S ] [-O ] [-k ] [-K ] [-Q] [-b] [-e] [-x] [-X] [-l ] [

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

labath added a comment. In D54692#1308207 , @zturner wrote: > In D54692#1308190 , @labath wrote: > > > Another reason for using libOption is that it is also usable as a parser > > for the lldb command line, whereas

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

zturner added a comment. In D54692#1308190 , @labath wrote: > > There’s actually been a slow push away from cl::opt. It’s less flexible > > and doesn’t support some things that the TableGen approach does. > > Recently there’s been a few efforts to port e

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

labath added a comment. > There’s actually been a slow push away from cl::opt. It’s less flexible > and doesn’t support some things that the TableGen approach does. > Recently there’s been a few efforts to port existing tools onto TableGen > options from cl::opt. > > I don’t think cl::opt is g

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

davide added a comment. In D54692#1308071 , @clayborg wrote: > Would be great to see old and new output like Zach suggested. Is there a > reason we need to use TableGen? Other command line tools just use llvm:🆑:opt > stuff. Seems a bit obtuse to use Tabl

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

zturner added a comment. I actually don't find this too bad, and like Pavel mentioned earlier, it's something that we can fix in LLVM and improve everyone's output. Which is one of the nice things about sharing code like this. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://revi

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere added a comment. Current output: https://reviews.llvm.org/P8117 Tablegen output: https://reviews.llvm.org/P8118 Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692

Re: [Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

There’s actually been a slow push away from cl::opt. It’s less flexible and doesn’t support some things that the TableGen approach does. Recently there’s been a few efforts to port existing tools onto TableGen options from cl::opt. I don’t think cl::opt is going away anytime soon so if it works I

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

clayborg added a comment. Would be great to see old and new output like Zach suggested. Is there a reason we need to use TableGen? Other command line tools just use llvm:🆑:opt stuff. Seems a bit obtuse to use TableGen? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

zturner added a comment. Can you post a short snippet of old help and new help output? Repository: rLLDB LLDB https://reviews.llvm.org/D54692 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere created this revision. JDevlieghere added a reviewer: zturner. JDevlieghere added a project: LLDB. Herald added subscribers: jfb, mgorny. Alternative to https://reviews.llvm.org/D54682. Repository: rLLDB LLDB https://reviews.llvm.org/D54692 Files: tools/driver/CMakeLists.txt