Re: [PATCH 2/6] Introduce `pre_expanded sals'

[Pedro Alves <pedro at codesourcery dot com>]

On Tuesday 12 April 2011 12:53:08, Jan Kratochvil wrote:
> On Tue, 12 Apr 2011 13:18:08 +0200, Pedro Alves wrote:
> > Hmm, doesn't sound right.  Conceptually, breakpoint locations are
> > multiple expansions of the same source location.  Different source locations
> > are different breakpoints.  E.g, bp_location doesn't have line
> > number or source file fields.  From the user's perpective, there's
> > only a single "point" in the source code for all the multiple locations
> > for a single breakpoint.
> 
> What about generalizing this so that breakpoint locations is a set of PCs
> matching the breakpoint expression?  

I disagree with that generalization.  E.g., next, you'll want that
a break on "C::C" (with C being a C++ class) sets a location on each
contructor overload).  And then breakpoints set in in-charge and
not-in-charge versions of each of those constructors ends up at the
same hierarchycal level under that super breakpoint.  If the user wants
to disable one of the (source level) overloads, he now needs to know
about the multiple locations of that specific overload.  I think you'd
want a "breakpoint group", with 3 levels of hierarchy.  Another argument,
is that frontends and users using them aren't expecting that a single
breakpoint is represented by more than one visual "point", circle next to
the sources, or something like that.  Hitting F8 to toggle a breakpoint's
enablement changing some other location source "point" enablement
in the sources not currently visible seems to break some abstration
to me.  I think such design change needs to consider all these
issues (and be experimented with some frontend).

I strongly suggest not relying on changing this as prerequisite
for stap support.

> Currently I do not understand how to fix:
> 	KFAIL: gdb.cp/re-set-overloaded.exp: start (GDB internal error) (PRMS: breakpoints/11657)
> 
> as currently breakpoint_re_set would need to create/delete user-visible
> breakpoint numbers.

Dunno.  Create new breakpoints or the extra overloads.  Or pick C::C() if it
exists, or any other contructor (or overload),  the first that matches, and
stick to it.  Or do consider generalizing like described above.

> 
> With the "new" (for some years) syntax `enable X.Y' ane `disable X.Y' one can
> control which breakpoints are active easily.
> 
> (gdb) break C::C
> Breakpoint 1 at 0x7d1: file ./gdb.cp/re-set-overloaded.cc, line 22.
> Breakpoint 2 at 0x7ba: file ./gdb.cp/re-set-overloaded.cc, line 21. (2 locations)
> warning: Multiple breakpoints were set.
> Use the "delete" command to delete unwanted breakpoints.
> (gdb) disable 2.2
> (gdb) info breakpoints 
> Num     Type           Disp Enb Address            What
> 1       breakpoint     keep y   0x00000000000007d1 in C::C(int) at ./gdb.cp/re-set-overloaded.cc:22
> 2       breakpoint     keep y   <MULTIPLE>         
> 2.1                         y     0x00000000000007ba in C::C() at ./gdb.cp/re-set-overloaded.cc:21
> 2.2                         n     0x00000000000007c4 in C::C() at ./gdb.cp/re-set-overloaded.cc:21
> 
> So that `set multiple-symbols ask' could be removed (probably completely incl.
> the `cancel' option).
> 
> I find the current two kinds of multiple breakpoints confusing to users (at
> least to myself).
> 
> 
> Thanks,
> Jan
> 

-- 
Pedro Alves