[PATCH v2] gdb/dap - dataBreakpointInfo & setDataBreakpoints

[Simon Farre]

> You probably should rebase this one on top of
>
> https://sourceware.org/pipermail/gdb-patches/2023-June/200252.html
>
> since that changes the breakpoint creation & reuse logic.  I suspect it
> would simplify your patch.
>
> Simon> +from .startup import send_gdb_with_response, in_gdb_thread, log
> Simon> +from .varref import find_variable
> Simon> +from .frames import frame_for_id, ScopedFrameOperation
> Simon> +class DataBreakpoint:
>
> Make sure to run 'black', I suspect it wants blank lines in here.
> I just do:
>
> murgatroyd. black gdb/python/lib/gdb/
I'll get it done in v3, I saw some minor documentation errors i had left 
behind too.
> Simon> +    def __init__(self, expr, type, condition=None, hit_condition=None, frameId = None):
>
> With the aforementioned series, this would probably be replaced by one
> of the transformer functions, which would then be type-checked.
I'll kill the comments about breakpoints being translated -  I wanted to 
be able to enable/disable individual breakpoint locations created by 
GDB, but that perhaps might not be the goal/scope of the 
DAP-interpreters initial feature? Besides, speaking strictly from a 
VSCode perspective here; one could always disable individual breakpoint 
locations using their "debug console"/evaluateRequest (which would be 
the equivalent of typing `disable break 1.1`), even if they don't show 
up in the UI. After having fiddled around with the breakpoint code, it'd 
require some potential hackery and I wasn't at all pleased with my 
intial attempt, so I'll remove the comments.

I'll re-write the watchpoint creation so that it can fit in the current 
design.
> Simon> +    # We've resolved the "path" for the variable using varrefs.  Construct it
> Simon> +    # for example: baz being a member variable of bar, which is a member of
> Simon> +    # foo: foo.bar.baz
> Simon> +    dataId = ".".join(reversed(path))
>
> Yeah, I'm not totally sure how this part should work.
>
> The issue I see is that variable references use python pretty-printers
> under the hood, but these work in a funny/limited way -- they were
> intended just for printing -- and so maybe won't interact well with
> watchpoints.  For one thing, nothing requires that the names returned by
> a printer be meaningful.  That is, they don't necessarily correspond to
> expressions that can actually access the member, and in practice do not
> (i.e., many C++ printers use readable labels but not usable ones, if
> that makes sense).
>
> I had two ideas for how to handle this situation.
>
> One is to keep using pretty printers, and just iterate over the children
> looking for one that matches the given name.  Then, if that printer
> returns a gdb.Value for the child, watch that value.  This would end up
> working like "watch -location".  If 'children' returns a string or
> whatever, it would just be unwatchable.
After having read this the first time I had no idea what you meant, but 
I (finally) figured out what you meant and where this approach would be 
greatly useful; for instance, if we had a `std::vector<Foo> foo` , we 
can now set `watch -location foo[10]` because we would be able to 
resolve element 10 and it's address, right?; using my approach it would 
attempt to watch `foo.[10]` (or maybe foo.10) which is nonsensical. So 
great idea, I'll try get something that'll work with pretty printers 
that return `gdb.Value`. I'll also add new tests for this as well.

I'll address the remainder of your review after I've addressed these 
points first.