[lldb-dev] Break setting aliases...

[Jim Ingham via lldb-dev]

> On Jul 23, 2020, at 1:51 AM, Pavel Labath <pavel at labath.sk> wrote:
> 
> On 22/07/2020 19:50, Jim Ingham wrote:
>>> On Jul 22, 2020, at 12:34 AM, Pavel Labath <pavel at labath.sk> wrote:
>>> 
>>> The "--" is slightly unfortunate, but it's at least consistent with our
>>> other commands taking raw input. We could avoid that by making the
>>> command not take raw input. I think most of the "modes" of the "b"
>>> command wouldn't need quoting in most circumstances -- source regex and
>>> "lib`func" modes being exceptions.
>> 
>> If anybody wants to work on this, I think Jonas is right, the first step would be to convert it to an actual command not a regex command.  The _regexp-break command is already hard enough to comprehend.
>> 
>> You could still do the actual specifier parsing with a series of regex’s if that seems best, though there might be easier ways to do it.  I also don’t think this would need to be a raw command, rather it could be a parsed command with one argument which was the breakpoint specifier and then all the other breakpoint flags.  
>> 
>> All the specifications you can currently pass to b are single words w/o spaces in them, or if they do have spaces they are the things you are used to having to quote in lldb: like file names with spaces.  
> The lib`func notation contains a backtick, which is used for expression
> substitution in the command interpreter. Currently we seem to be just
> dropping an unmatched backtick, which would break that.  We could change
> it so that the unmatched backtick is kept, though I would actually
> prefer to make that an error..
> 

IMO the backtick parsing in lldb is currently done incorrectly.  In the original design, it was supposed to be another form of quote, not a preprocessing stage.  I think, for instance, this is more surprising than useful:

(lldb) run
Process 44292 launched: '/tmp/a.out' (x86_64)
Process 44292 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
    frame #0: 0x0000000100003f66 a.out`main at variable.c:7
   4   	main()
   5   	{
   6   	  int a = 100;
-> 7   	  printf("%d\n", a);
    	                 ^
   8   	  return 0;
   9   	}
Target 0: (a.out) stopped.
(lldb) expr char *$my_str = "some `a` other"
(lldb) expr $my_str
(char *) $my_str = 0x00000001001423e0 "some 100 other"

And this has tripped people up in the past.  

The intent was to substitute an option value or argument with the result of an expression if it was appropriately marked.  If backticks worked that way an inter-word backtick would not be significant.  

We could also change the character we print for separating shlib from file name both in how we print the spec and in how we encode it in ‘b’.  As long as these two are consistent I don’t think folks would much care what it was...


> 
>>> "br set" help starts with a long list command switches, which are
>>> supposed to show which options can be used together. I think this sort
>>> of listing is nice when the command has a couple of modes and a few
>>> switches, but it really misses the mark when it ends up listing 11 modes
>>> with approximately 20 switches in each one.
>>> 
>>> This is then followed by descriptions of the 20 or so switches. This
>>> list is alphabetical, which means the most commonly used options end up
>>> burried between the switches I've never even used.
>> 
>> Yes.  I’ve said many times before that making “break set” the master command for breakpoint setting was a mistake.  ...
> 
> 
> Restructuring the commands is one thing. It might be a good idea but
> there are less invasive things we could do to make this better. Just
> restructuring the help output to make the most common use cases easier
> to find would help a lot IMO. We could drop or simplify the "synopsis"
> section, maybe replacing it with a couple of examples of the most useful
> kinds of breakpoints.
> 
> Then we could group the options to keep the similar ones together and
> make them easier to find/skip. Maybe with groups like:
> - options specifying where to set a breakpoint: --file, --line; --name; etc.
> - options restricting the reported breakpoint hits:
> --thread-id,--thread-name,--condition, etc.
> - various modifiers: --disable, --hardware, --one-shot, etc.
> - others (?)
> 
> The division may not be perfect (like, is --ignore-count a "modifier" or
> does it "restrict breakpoint hits"?), but even so I think this would
> make that list a lot easier to navigate. But we digress...

This is already partly done.  The primaries for all these settings are always listed first, because they are required for a given form.  So you see:

Command Options Usage:
  breakpoint set [-DHd] -l <linenum> [-G <boolean>] [-C <command>] [-c <expr>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-R <address>] [-N <breakpoint-name>] [-u <column>] [-f <filename>] [-m <boolean>] [-s <shlib-name>] [-K <boolean>]
  breakpoint set [-DHd] -a <address-expression> [-G <boolean>] [-C <command>] [-c <expr>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-N <breakpoint-name>] [-s <shlib-name>]
  breakpoint set [-DHd] -n <function-name> [-G <boolean>] [-C <command>] [-c <expr>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-R <address>] [-N <breakpoint-name>] [-f <filename>] [-L <source-language>] [-s <shlib-name>] [-K <boolean>]

etc.  The first entries are always the primaries - how you are setting the breakpoint.  The first one is a little odd because it only lists -l.  That’s because the file is not required when setting file&line breakpoints (it uses the default file if you don’t specify one.)  But the rest mostly make sense.  However, you have to know what you are looking for to see this.  I’m not sure reordering the other options will help this particular issue much.

OTOH, it would be really useful to maintain OptionGroups when the command options are consed up, and represent this in the help output.  Right now, you flatten all the options groups your command uses into one list and that gets sorted and printed.  But the OptionGroups do express some of the ordering you are looking for.  For instance all the options that “break set” and “break modify” share, i.e. all the ones that effect how you react to hitting a breakpoint rather than how you set it, are in their own OptionGroup.

> 
> On 22/07/2020 20:20, Greg Clayton wrote:
>> BTW: to see what things expand to after reach regex alias, just set
> this setting first:
>> 
>> (lldb) settings set interpreter.expand-regex-aliases true
> 
> That is cool. I wonder if that should be the default...

I don’t think so.  People don’t really want to know how the b command works, they just want it to magically work.  And my experience is that people really hate extra output.  We had a knock-down drag-out fight about the fact that the “po” command used to print the result variable of the object whose “object description” was being printed.  So the output was:

(lldb) po foo
(NSWhatever *) $1 = 0x123454
This object is a really great object, use it a lot

And though the extra info was in fact useful, the majority opinion was the people didn’t want to have to look past it for the po result (*).  This was not a terribly politely expressed opinion either...

I think if we spit out some text people didn’t understand every time they used the “b” command we would have a similar reaction.

Jim
 
(*) Then of course the people who did know what it meant and were using the result variable as well complained, so now we have the oddly named “description-verbosity” option to expr to turn it back on again.

> 
> pl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20200723/9181252e/attachment-0001.html>