Re: [PATCH:MI] Use observers for breakpoints

[Nick Roberts <nickrob at snap dot net dot nz>]

 > >  > > (gdb)
 > >  > > -break-watch i22
 > >  > > =breakpoints-changed,bkpt={number="4",type="watchpoint",disp="keep",e
 > >  > >nabled="y",addr="",what="i22",times="0",original-location="i22"}
 > >  > > ^done,wpt={number="4",exp="i22"}
 > >  >
 > >  > Is this an output we actually want? ;-)
 > >  > With one breakpoint mentioned twice?
 > > 
 > > I don't know why watchpoints are currently reported in a different way to
 > > normal breakpoints or why type="watchpoint" isn't adequate and a special
 > > field "wpt" is needed, but I would suggest removing the latter output.
 > 
 > Good. You did not mention this initially, so I wondered if you consider
 > this OK. The 'wpt' comes from the mention function.

I'm just trying to find agreement the MI should use event notification for
change of state and not directly MI command output.  I should have used RFC in
the subject header.  I will submit a proper patch later.  Note that removing
the "wpt" field requires a new MI level.


 > ...
 > > So is update_global_location_list just updating the target, i.e. putting
 > > new breakpoints in, old ones back etc?  If so, perhaps changes in the
 > > breakpoint list have already been reflected elsewhere and no observer
 > > calls need to be made here.
 > 
 > Depends on your design. I think it makes perfect sense to report changes in
 > properties that affect inserted breakpoints in update_global_location_list,
 > and report changes in other properties in the corresponding functions
 > (condition_command, etc).

When I submit a patch you'll probably have to explain to me where breakpoints
might change.


 > >  > Is there any way to make -break-insert report the new breakpoint
 > >  > directly, as opposed to via notification?
 > > 
 > > That's what it currently does, isn't it?  I'm trying to move away from
 > > that so that GDB just reports changes no matter how they are made, e.g.,
 > > MI command or CLI command.
 > 
 > I think that for -break-insert, saying "^done" and then saying, "ah, btw,
 > some breakpoints were inserted", is a bit indirect. For example, in
 > KDevelop, there's a class to represent breakpoint. When a breakpoint is
 > created, and -break-insert is sent, and breakpoint is inserted, the 'id'
 > field of that class is set to breakpoint number that gdb reports, and at
 > this point we know that the breakpoint is correctly inserted. If
 > -break-insert no longer reports the breakpoint directly, I'd have to somehow
 > match the output =breakpoints-changes to the previous -break-insert. If I
 > don't, then =breakpoints-changed will actually add new breakpoint to the
 > breakpoint table, without marking the breakpoint been inserted via
 > -break-insert as inserted.
 > 
 > One can argue that such matching is not very hard to do. But if so, then
 > it's not hard to do for GDB, either, and -break-insert can still report the
 > breakpoint that was inserted.

The event notification reports the breakpoint number so what is there to
match?.  I don't see why it should be reported twice, except perhaps not to
break existing behaviour.  In that case, I think it would be better to
condition on the MI level, so current level outputs as now and mi=3 outputs
notifications only.


 > I'm also not sure that emitting =breakpoints-changes in response to
 > -break-condition, or -break-disable, is a good idea. It does not add any new
 > information.

In a frontend that uses the console, e.g., Eclipse, if the user enters a
command like "disable 1", how does it know that this breakpoint has been
disabled?


 > Presumably, we can suppress the breakpoint observer during executing of MI
 > commands that directly create/modify breakpoint,

That seems to defeat the object.

 >                                                    and leave the observer
 > enabled for other commands -- where we don't mean no breakpoint changes but
 > might get them.

Where would that happen?

 > >  > I think that except for compatibility issues due to -break-insert no
 > >  > longer reporting the breakpoint that is created as part of ^done
 > >  > response, this patch is good.
 > > 
 > > The only way I can see around compatibility issues would be to add a new
 > > level for MI.
 > 
 > Or make -break-insert output the new breakpoint directly, by temporary
 > suppressing the observer.

I guess it could output the breakpoint directly in addition to the notification.
Current frontends should ignore such notifications (if they don't they will
presumably break with the ones already added).


 > > This change could be lumped with others that are being proposed, 
 > > e.g, the no stop mode.  
 > 
 > Just to clarify -- the non stop mode is optional, and so does not introduce
 > any backward compatibility issues.

I'm thinking of changes to accommodate non stop mode, e.g., no token in stopped
output (I realise that this has already been made).  Note these examples are
still present in the GDB/MI Program Execution node of th manual:

     (gdb)
     111-exec-continue
     111^running

     (gdb)
     000-exec-run
     000^running

changing ^running to *running?, removing the (gdb) `prompt'?


 > > Then the logic could be separated according to MI level and notice given
 > > that at some stage the old level would be removed.
 > > 
 > > It's a lot of work and I must admit I'm not really sure that I have the
 > > stamina to go through with all of it.
 > 
 > It seems to me that this patch, with minor adjustments, can be checked in
 > soonish.

I'll see what I can do but a lot of questions seem to remain.

-- 
Nick                                           http://www.inet.net.nz/~nickrob