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

Simon Marchi
2019-01-11T23:36:26Z

On 2019-01-11 1:39 p.m., Pedro Alves wrote:
> On 01/11/2019 12:34 PM, Andrew Burgess wrote:
>> * Simon Marchi <simon.marchi@ericsson.com> [2019-01-11 00:15:34 +0000]:
>>
>>> [CCing Pedro because we had some discussions earlier about that offline]
>
>
> Thanks. This was also recently-ish discussed in PR9659.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=9659

Ahh thanks for the reference, I couldn't remember where you had already
wrote about that.

> My original concern with MI bumps for individual MI fixes is that they
> force an all-or-nothing approach on the frontends. Let me expand.
>
> Suppose a frontend developer only cares about the multi-location
> fix, and not any of the other (supposed) fixes that go into MI3 that
> make it backwards incompatible. It was with that in mind that I had
> suggested at <https://sourceware.org/bugzilla/show_bug.cgi?id=9659#c20>
> to consider going with the "-fix-break-list-bug" solution first.

I agree this would be nice.

> That would be usable with MI2 and also be enabled by default with
> MI3 (with no way to disable). Then later on, when we get rid of
> MI2, the "-fix-break-list-bug" setting disappears.

Well this addresses my concern that frontends won't need to use
-fix-break-list-bug until the end of time, so I am ok with it.

> But I suppose that that's really an unnecessary complication if we're
> not really going to massively change MI every other release, and if
> migrating a frontend to a new MI version isn't expected to be that
> complicated. We probably aren't and it probably isn't.

I'll at least give it a try, implementing it is probably not hard. If it doesn't
add too much maintenance burden, I'm not against it. If I do it for this bug, it
will pave the path for future bug fixes, so hopefully it will be smoother next time.

> So all things considered, it's fine with me to go your route directly
> without a "-fix-break-list-bug" step.

As I said, I'll give it a try. I intend to name it -fix-multi-location-breakpoint-output.

> I agree with Andrew below though. Bumping the MI version this late in
> the cycle is probably not a good idea.

I agree, I intend to merge a fix for this after 8.3 has branched.

> If we want to fix this bug for 8.3, we could merge the fix while
> leaving MI2 as the default, declare MI3 stable, and then bump the
> WIP MI version to MI4. I.e., the comments in the code that talk
> about things to fix for MI3 should become references to MI4 instead.

Yes, although I would wait until 8.3 is branched before merging it.

Btw I realized the output with this patch is not good. For -break-list with two multi-location
breakpoints, it results in something like:

body=[
bkpt={ ... },
locations={ ... },
bkpt={ ... },
locations={ ... },
]

Where I was aiming for:

body=[
bkpt={
...,
locations={ ... },
},
bkpt={
...,
locations={ ... },
},
]

The next version will fix this.

Simon