Re: [PATCH] Fix MI output for multi-location breakpoints

[Simon Marchi <simon.marchi@ericsson.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4872 bytes --]

On 2019-01-11 3:04 a.m., Eli Zaretskii wrote:
>> From: Simon Marchi <simon.marchi@ericsson.com>
>> CC: "palves@redhat.com" <palves@redhat.com>, Simon Marchi	<simon.marchi@ericsson.com>
>> Date: Fri, 11 Jan 2019 00:15:34 +0000
> 
> The documentation changes are approved with these comments:
> 
>> gdb/doc/ChangeLog:
>>
>> 	* gdb.texinfo (Choosing Modes): Mention mi3.
>> 	(Command Interpreters): Likewise.
> 
> You seem to be using the chapter/section names in the parentheses.
> You should be using node names instead (or use Emacs, which will do
> that for you ;-).

Oh, that's what I have always done!  Is this better?

	* gdb.texinfo (Mode Options): Mention mi3.
	(Interpreters): Likewise.

> 
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 4a00834d0bf..a422a395c47 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -1268,12 +1268,11 @@ program or device.  This option is meant to be set by programs which
>>  communicate with @value{GDBN} using it as a back end.
>>  @xref{Interpreters, , Command Interpreters}.
>>  
>> -@samp{--interpreter=mi} (or @samp{--interpreter=mi2}) causes
>> +@samp{--interpreter=mi} (or @samp{--interpreter=mi3}) causes
>>  @value{GDBN} to use the @dfn{@sc{gdb/mi} interface} (@pxref{GDB/MI, ,
>> -The @sc{gdb/mi} Interface}) included since @value{GDBN} version 6.0.  The
>> -previous @sc{gdb/mi} interface, included in @value{GDBN} version 5.3 and
>> -selected with @samp{--interpreter=mi1}, is deprecated.  Earlier
>> -@sc{gdb/mi} interfaces are no longer supported.
>> +The @sc{gdb/mi} Interface}).  The @sc{gdb/mi} interfaces 1 and 2 are
>> +available, but deprecated.  Earlier @sc{gdb/mi} interfaces are no
>> +longer available.
> 
> Please don't remove the version information from this text.  At the
> very least, we should tell what GDB version introduced the latest mi3
> syntax.  We should tell this here, and not only in the "Interpreters"
> node, because this section is a concise list of invocation options,
> and should include the important information without sending the
> reader to read the more detailed parts.

The reason I thought this was not really appropriate here is that this manual
applies to a particular version of GDB (e.g. it's the manual shipped with
GDB 9.1).  So if you are using that version of GDB, it doesn't really matter
which version of GDB introduced mi3, all that matters is that it exists now.

As you said, this section is a concise list of invocation options.  But I don't
consider this very important information to have at the fingertips.

As Andrew suggested, I think we should have a list or table of all mi versions,
when they were introduced, and what the breaking changes are.  This would help
people writing or upgrading their MI frontend.   But I don't think that the
casual user looking for the possible command line flags will need to know the
history of MI.

Finally, the goal is to reduce the duplication of information, so there is less
things to update when releasing a new MI version (therefore less chance of having
outdated information).

>> +@item mi3
>> +@cindex mi3 interpreter
>> +The @sc{gdb/mi} interface introduced in @value{GDBN} 9.  It is the latest
>                                            ^^^^^^^^^^^^^^
> GDB 9.1, presumably?

Right.

>>  @item mi1
>>  @cindex mi1 interpreter
>> -The @sc{gdb/mi} interface included in @value{GDBN} 5.1, 5.2, and 5.3.
>> +The @sc{gdb/mi} interface introduced in @value{GDBN} 5.1.
> 
> I think the old text is better.

The problem I see with the current wording is that it implies that mi1 is included
in version 5.1, 5.2 and 5.3 only.  In reality, it's included in all versions from
5.1 and up.  So it seems odd to list 5.2 and 5.3 specifically.  Do you have a
suggestion to address this?

> Thanks.
> 
> P.S. I wonder how did we let this problem slip through the cracks when
> multiple-location breakpoints were introduced?  Maybe we should do
> something to avoid such mistakes in the future.  We really shouldn't
> be changing the MI syntax in incompatible ways so late into GDB
> development cycle.

Well it's been known for a long time, as shows this bug from 2008 (AFAIU,
multiple location breakpoints were introduced around 2007?):

https://sourceware.org/bugzilla/show_bug.cgi?id=9659

I've been meaning to fix this for a few years, but always put it aside (until now)
because of the headache introducing backwards-incompatible MI changes represents.

Note that I intend to merge this change (or whatever solution we come up with) after
the 8.3 branch is created, so it will be part of the next release cycle.  We should
have plenty of time to iron out any bug we find, as well as introduce other MI-breaking
changes, while at it.

Simon
\x16º&Öéj×!zÊÞ¶êçם÷ë™b²Ö«r\x18\x1dn–­r\x17¬