Re: [PATCH 2/8] gdb: don't duplicate 'thread' field in MI breakpoint output

[Eli Zaretskii <eliz@gnu.org>]

> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Mon, 20 Feb 2023 14:13:47 +0000
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> 
> When creating a thread-specific breakpoint with a single location, the
> 'thread' field would be repeated in the MI output.  This can be seen
> in two existing tests gdb.mi/mi-nsmoribund.exp and
> gdb.mi/mi-pending.exp, e.g.:
> 
>   (gdb)
>   -break-insert -p 1 bar
>   ^done,bkpt={number="1",type="breakpoint",disp="keep",
> 	      enabled="y",
> 	      addr="0x000000000040110a",func="bar",
> 	      file="/tmp/mi-thread-specific-bp.c",
> 	      fullname="/tmp/mi-thread-specific-bp.c",
> 	      line="32",thread-groups=["i1"],
> 	      thread="1",thread="1",  <================ DUPLICATION!
> 	      times="0",original-location="bar"}
> 
> I know we need to be careful when adjusting MI output, but I'm hopeful
> in this case, as the field is duplicated, and the field contents are
> always identical, that we might get away with removing one of the
> duplicates.
> 
> The change in GDB is a fairly trivial condition change.
> 
> We did have a couple of tests that contained the duplicate fields in
> their expected output, but given there was no comment pointing out
> this oddity either in the GDB code, or in the test, I suspect this was
> more a case of copying whatever output GDB produced and using that as
> the expected results.  I've updated these tests to remove the
> duplication.
> 
> I've update lib/mi-support.exp to provide support for building
> breakpoint patterns that contain the thread field, and I've made use
> of this in a new test I've added that is just about creating
> thread-specific breakpoints and checking the results.  The two tests I
> mentioned above as being updated could also use the new
> lib/mi-support.exp functionality, but I'm going to do that in a later
> patch, this was it is clear what changes I'm actually proposing to
> make to the expected output.
> 
> As I said, I hope that frontends will be able to handle this change,
> but I still think its worth adding a NEWS entry, that way, if someone
> runs into problems, there's a chance they can figure out what's going
> on.
> 
> This should not impact CLI output at all.
> ---
>  gdb/NEWS                                      |  8 +++
>  gdb/breakpoint.c                              |  2 +-
>  gdb/testsuite/gdb.mi/mi-nsmoribund.exp        |  2 +-
>  gdb/testsuite/gdb.mi/mi-pending.exp           |  2 +-
>  gdb/testsuite/gdb.mi/mi-thread-specific-bp.c  | 44 +++++++++++++++++
>  .../gdb.mi/mi-thread-specific-bp.exp          | 49 +++++++++++++++++++
>  gdb/testsuite/lib/mi-support.exp              | 32 ++++++++----
>  7 files changed, 127 insertions(+), 12 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.mi/mi-thread-specific-bp.c
>  create mode 100644 gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp

Thanks, the NEWS part is OK.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>