Re: [PATCH] gdb/mi: New commands to catch C++ exceptions

[Eli Zaretskii <eliz@gnu.org>]

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Thu,  9 May 2019 01:05:00 +0100
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 288615b8cd3..e350a18f1f2 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -27,6 +27,10 @@
>       'array_indexes', 'symbols', 'unions', 'deref_refs', 'actual_objects',
>       'static_members', 'max_elements', 'repeat_threshold', and 'format'.
>  
> +* MI changes
> +
> +  ** New command -catch-throw, -catch-rethrow, and -catch-catch.
            ^^^^^^^
That should be "commands", right?

> +@node C++ Exception GDB/MI Catchpoint Commands
> +@subsection C++ Exception @sc{gdb/mi} Catchpoints

@node cannot have markup, but @subsection can.  Our convention is to
use C@t{++} for "C++", as that makes it look prettier in print.
Please use this markup in @subsection and elsewhere in the text.

> +@smallexample
> + -catch-throw [ -t ] [ -r @var{regexp}]
> +@end smallexample
> +
> +Catch the throwing of a C@t{++} exception.

"Catch the throwing" is ambiguous, because the meaning of "catch" here
is unclear.  The section of the manual that describes the
corresponding CLI commands uses a more clear description: "Stop when
@var{event} occurs."  So I suggest to use the same:

  Stop when the debuggee throws a C@t{++} exception.

>                                               If @var{regexp} is given
> +then only exceptions whose type matches the regular expression will be
> +caught.

A comma is missing after "given".

> +If @samp{-t} is given then the catchpoint is enabled only for one

Likewise.

> +stop, the catchpoint is automatically deleted after the first event is
> +caught.

Again, please don't use "caught", but instead "after stopping once for
the event."

> +The possible optional parameters for this command are:
> +
> +@table @samp
> +@item -r @var{regexp}
> +Use @var{regexp} to match against the exception type.
> +@item -t
> +Create a temporary catchpoint.
> +@end table

This part sounds like a repetition to me.

> +@subsubheading @value{GDBN} Command
> +
> +The corresponding @value{GDBN} commands are @samp{catch throw}
> +and @samp{tcatch throw}.

Please add a cross-reference to where these CLI commands are
described.

> +@subsubheading Example
> +
> +@smallexample
> +-catch-throw -r exception_type
> +^done,bkptno="1"
> +(gdb)
> +@end smallexample

IMO, this example is not useful, because it doesn't show the effect of
the used features.  How about adding the output when the program is
stopped because it throws a matching exception?  Then you'd be able to
show that the type is matched as a regexp, not as a literal string.

> +@smallexample
> + -catch-rethrow [ -t ] [ -r @var{regexp}]
> +@end smallexample
> +
> +Catch the rethrowing of a C@t{++} exception.  If @var{regexp} is given
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"Stop when a C@t{++} exception is re-thrown."

A comma is missing after "given".

> +If @samp{-t} is given then the catchpoint is enabled only for one

Likewise.

> +The possible optional parameters for this command are:
> +
> +@table @samp
> +@item -r @var{regexp}
> +Use @var{regexp} to match against the exception type.
> +@item -t
> +Create a temporary catchpoint.
> +@end table

Repetition again.

> +@subsubheading @value{GDBN} Command
> +
> +The corresponding @value{GDBN} commands are @samp{catch rethrow}
> +and @samp{tcatch rethrow}.

Again, please add a cross-reference.

> +@smallexample
> +-catch-rethrow -r exception_type
> +^done,bkptno="1"
> +(gdb)
> +@end smallexample

Same comment as for the previous example.

> +@smallexample
> + -catch-catch [ -t ] [ -r @var{regexp}]
> +@end smallexample
> +
> +Catch the catching of a C@t{++} exception.  If @var{regexp} is given

Same comments as above.

Btw, as the text you use to describe all 3 commands is extremely
similar, maybe you should describe them in a single subheading,
instead of copying the same text 3 times?

Thanks.