This changes the driver to use libOption for option parsing. This means that option are now generated from a tablegen file.
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?
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.
I'm afraid this is not true. lld uses TableGen and that was a big success. See https://github.com/llvm-mirror/lld/blob/master/ELF/Options.td (considering that linkers have to handle all the annoying cmdline compatibility with gcc, e.g. options which take single or double underscores).
FWIW, I'm in favour of this change.
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 don’t
have a strong opinion, but it’s kinda nice to standardize on “the one
true method” if that’s the direction things are heading anyway
Another reason for using libOption is that it is also usable as a parser for the lldb command line, whereas cl::opt is definitely not (it uses global variables). And there's value in consistency between the lldb driver and the built-in command line.
This is true too. Although I believe libOption doesn't support subcommands, which would be required in order to use it for the interactive lldb command line, but again, there would be value in adding that to libOption outside of llvm (cl::opt supports it, so it's required in order to port some remaining llvm tools to libOption)
Adding subcommands is one way. Another would be to simply keep the existing subcommand-parsing code (which we already have, as getopt doesn't support that either), and just replace the getopt part with libOption.
the old usage:
Usage: lldb -h lldb -v [[--] <PROGRAM-ARG-1> [<PROGRAM_ARG-2> ...]] lldb -a <arch> -f <filename> [-c <filename>] [-s <filename>] [-o <none>] [-S <filename>] [-O <none>] [-k <none>] [-K <filename>] [-Q] [-b] [-e] [-x] [-X] [-l <script-language>] [-d] [-z <filename>] [[--] <PROGRAM-ARG-1> [<PROGRAM_ARG-2> ...]] lldb -n <process-name> -w [-s <filename>] [-o <none>] [-S <filename>] [-O <none>] [-k <none>] [-K <filename>] [-Q] [-b] [-e] [-x] [-X] [-l <script-language>] [-d] [-z <filename>] lldb -p <pid> [-s <filename>] [-o <none>] [-S <filename>] [-O <none>] [-k <none>] [-K <filename>] [-Q] [-b] [-e] [-x] [-X] [-l <script-language>] [-d] [-z <filename>] lldb -P lldb -r [<none>] -R <none>
Showed how all the options can be used together. It would be nice to group the options into the following groups:
- general options (--arch, --version, --help, --file, --editor, --no-use-colors, -debug, --reproducer, --repl*)
- scripting options (--python-path, --script-language)
- command options (--no-lldbinit, --batch, --one-line*, --source*)
- attach options (--attach-name, --attach-pid, --wait-for)
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. That's not that big a deal for the lldb driver command, as it doesn't have all that many disparate groups of options. But Davide mentioned using this for the lldb command line as well, where we do use this to help auto-validate complex commands in a fair number of commands.
One thing that is missing from the SB API's was some way of defining the arguments and options for a command. Because of this, Python based commands can't take part in the completions features and auto-help generation that "real" commands had. If we had such a thing, then the driver could use the same mechanism to define itself as a command, and then just run itself at startup. That would centralize all the argument parsing that we do regardless of how it is backed.
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 relying on this. For example consider what happens when you say
lldb -o 'do one thing' -s 'source/some/file'
You'd expect that the one-line is executed before sourcing the file. This wasn't the case with the previous version of this patch and is now fixed.
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 options. Ditto for the -o's, -s's and -q's (except they get run after the file). I don't know how complicated a set of tests I wrote when I originally added these, so you might want to check that all these cases are covered.
|876–888 ↗||(On Diff #175377)|
I am not entirely thrilled by the hard-coding of the option combinations here. It sounds like the kind of thing that will invariably get out of sync (I think it would be better to just not have it). Instead of trying to exhaustively list all possible option combinations (which I generally find too long to make sense of), and still getting it wrong (Why is there a [[--] <PROGRAM-ARG-1> [<PROGRAM_ARG-2> ...]] after -v in the second option form?), maybe it would be better to just output a short prose here explaining the general principle. Maybe something like "LLDB can be started in several modes. Passing an executable as a positional arguments prepares lldb to debug the given executable. Using one of the attach options causes lldb to immediately attach to the given process. Command options can be combined with either mode and cause lldb to run the specified commands before starting the interactive shell. Using --repl starts lldb in REPL mode. etc."
However, if everyone is happy with the current approach then I won't stand in the way..
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 --arch x86_64 /path/to/program/a.out -- --arch arvm7
The first arch is for lldb, and any options past the -- are for the inferior program
Attaching examples, loading a core file, printing python path, maybe specifying some lldb commands with -o and -O to show how those work.
- Add EXAMPLES section
EXAMPLES: The debugger can be started in several modes. Passing an executable as a positional argument prepares lldb to debug the given executable. Arguments passed after -- are considered arguments to the debugged executable. lldb --arch x86_64 /path/to/program -- --arch arvm7 Passing one of the attach options causes lldb to immediately attach to the given process. lldb -p <pid> lldb -n <process-name> Passing --repl starts lldb in REPL mode. lldb -r Passing --core causes lldb to debug the core file. lldb -c /path/to/core Command options can be combined with either mode and cause lldb to run the specified commands before or after events, like loading the file or crashing, in the order provided on the command line. lldb -O 'settings set stop-disassembly-count 20' -o 'run' -o 'bt' lldb -S /source/before/file -s /source/after/file lldb -K /source/before/crash -k /source/after/crash
I noticed a small problem, this change breaks "lldb -c <corefile>". The inline comment explains the root cause.
there's a small bug in here: optarg is the global definition, I assume the intention was to use the getValue() instead. as is, it breaks "lldb -c <corefile>"
it would be nice to get rid the global optarg as well since it masks these kind of mistakes.