Re: [PATCH v4 9/9] enable target-async

[Tom Tromey <tromey@redhat.com>]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Sorry, I wasn't clear.  I understand what as a whole the patch is trying
Pedro> to do; what I don't understand is why a hack was necessary, or its
Pedro> implementation.  E.g., what exactly fails if the hack is not
Pedro> in place?; Why this spot for the hack?; What's the predicate used in
Pedro> the hack actually checking?

I don't remember, but I'll back it out and redo the analysis to find
out.

Pedro> Now that I looked again a little closer, I recalled that GDB in
Pedro> sync mode always outputs a silly extra prompt right after
Pedro> ^running (in response to execution commands), before the target
Pedro> stops, even though GDB is not ready for input then.  Guess this
Pedro> hack is related?

Probably so.  To my surprise dealing with the prompt was by far the most
difficult part of this series.  Doubly sad since the prompt just seems
like a mistake to me in the first place.

>> set target-async on
>> &"set target-async on\n"
>> ^done
>> (gdb) 
>> -list-target-features
>> ^done,features=["async"]

Pedro> I guess we could see it either way.  -list-target-features lists
Pedro> target features.  So with GDB today, until "set target-async on"
Pedro> is issued, the target doesn't support async.  After the series,
Pedro> the target does support async even if MI itself isn't async.
Pedro> E.g., I'd've expected 'interpreter-exec mi "-list-target-features"'
Pedro> issued from the cli to list "async".

Pedro> Given the chicken and egg thing already exists today, this makes
Pedro> me think no frontend is actually looking for this feature... (?)

Pedro> Anyway, fine with me to leave this as you have it for now, and
Pedro> maybe reconsider it after the series is in.

Yeah.  My overall goal for this series was to eliminate the need for
"set target-async on", while at the same time preserving the current MI
output, regardless of whether I felt it was correct; unless said change
is clearly compatible according to the normal MI rules.  My reasoning is
that this is the best way to avoid breaking a client.

>>>> -# so the stop reason is printed into MI uiout an.
>>>> -if {$async} {
>>>> -    set reason "end-stepping-range"
>>>> -} else {
>>>> -    set reason ""
>>>> -}
>>>> +set reason "end-stepping-range"
>> 
Pedro> I'm a little confused by this one.  Isn't it still necessary
Pedro> for targets that don't do async?

I re-examined this and I think the answer is much simpler than all the
incorrect things I wrote before.

git master gdb in the default (target-async off) mode prints:

*stopped,frame={addr="0x0000000000400597",func="callee4",args=[],file="../../../binutils-gdb/gdb/testsuite/gdb.mi/basics.c",fullname="/home/tromey/Space/SecondArcher/binutils-gdb/gdb/testsuite/gdb.mi/basics.c",line="26"},thread-id="1",stopped-threads="all",core="0"

The gdb from this branch prints:

*stopped,reason="end-stepping-range",frame={addr="0x0000000000400597",func="callee4",args=[],file="../../../binutils-gdb/gdb/testsuite/gdb.mi/basics.c",fullname="/home/tromey/gnu/gdb/devel/binutils-gdb/gdb/testsuite/gdb.mi/basics.c",line="26"},thread-id="1",stopped-threads="all",core="3"


That is, gdb now emits reason="end-stepping-range".  This is a
compatible change to the output.  This explains the patch: now the
!$async case is no longer hit.

Tom