Re: [PATCH] python/19506 -- gdb.Breakpoint address location regression

[Joel Brobecker <brobecker@adacore.com>]

> [Apologies for waxing a little philosophical here...]
[Apologies for triggering the discussion ;-)...]

> For me, #1 really boils down to UI writers "passing the buck" down to
> gdb's internals (via CLI generalisms). With the location API work, I
> tried to make this layer a little more distinct/separate. UIs are
> responsible for implementing their own representations of locations in a
> way most consistent with their implementation specifics. [Example: using
> 'gdb.Breakpoint("-source foo.c -line 3")' seems downright wrong to me.]

Agreed.

> In light of this, I am suggesting that instead of requiring
> python/mi/guile/whatever to implement their own string_to_event_location
> functions, that the current string_to_event_location be split into a
> "legacy" portion (that deals with address, probe, and linespec
> locations) and a newer (and separate) explicit-handling form (for the
> CLI only).
> 
> This leaves python/guile to implement an explicit location specification
> in a manner more consistent with those interpreters, e.g., python could
> use named arguments to gdb.Breakpoint:
> 
>   gdb.Breakpoint(source="foo.c", line="3")

I agree that each extension language should have their own clean API
for creating breakpoints using explicit locations. That's a lot
cleaner.

> This way we wouldn't "force" these languages to use the CLI's
> argument/value paradigm. I've already done something similar for MI. [MI
> is currently using string_to_event_location, which means it will
> erroneously support the CLI's explicit syntax in addition to its own!
> Bug! My bad! Have patch.]
> 
> In short:
> 1) Remove explicit locations from string_to_event_location.
>    Use this "new" function /everywhere/ legacy linespec support is
>    desired. In my working tree, I call this
>    string_to_event_location_basic.
> 2) string_to_event_locations can now be refactored to do two things:
>    check for an explicit location OR call the _basic version to deal
>    with linespec, address, and probe locations.
> 
> WDYT? [I can send a small patch series to address/clean up all of these,
> if you think this is a reasonable resolution.]

It sounds like a good idea, as I think it'll factorize the code.
In the grand scheme of things, the current situation is not all
that bad, though, so I wouldn't say that this absolutely needs
to be done ahead of everything else.

So, what I would suggest is first push the current patch if
people agree with it. That way, we are covered for the release
branch. And then, if you have some time to look at this
enhancement before we release 7.11, then we can look at
possibly backporting it.

Did anyone review the patch? Typically, Doug is the one reviewing
those patches, but I can take a look as well.

Thanks, Keith!
-- 
Joel