Add block['var'] accessor
Commit Message
Currently we support iteration on blocks; this patch extends that to make
subscript access work as well.
gdb/ChangeLog:
2019-08-01 Christian Biesinger <cbiesinger@google.com>
* NEWS: Mention dictionary access on blocks.
* python/py-block.c (blpy_getitem): New function.
(block_object_as_mapping): New struct.
(block_object_type): Use new struct for tp_as_mapping field.
gdb/doc/ChangeLog:
2019-08-01 Christian Biesinger <cbiesinger@google.com>
* python.texi (Blocks In Python): Document dictionary access on blocks.
gdb/testsuite/ChangeLog:
2019-08-01 Christian Biesinger <cbiesinger@google.com>
* gdb.python/py-block.exp: Test dictionary access on blocks.
---
gdb/NEWS | 3 +++
gdb/doc/python.texi | 7 +++++-
gdb/python/py-block.c | 32 ++++++++++++++++++++++++++-
gdb/testsuite/gdb.python/py-block.exp | 5 +++++
4 files changed, 45 insertions(+), 2 deletions(-)
Comments
> Date: Thu, 1 Aug 2019 17:34:20 -0500
> From: "Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>
> Cc: Christian Biesinger <cbiesinger@google.com>
>
> Currently we support iteration on blocks; this patch extends that to make
> subscript access work as well.
>
> gdb/ChangeLog:
>
> 2019-08-01 Christian Biesinger <cbiesinger@google.com>
>
> * NEWS: Mention dictionary access on blocks.
> * python/py-block.c (blpy_getitem): New function.
> (block_object_as_mapping): New struct.
> (block_object_type): Use new struct for tp_as_mapping field.
>
> gdb/doc/ChangeLog:
>
> 2019-08-01 Christian Biesinger <cbiesinger@google.com>
>
> * python.texi (Blocks In Python): Document dictionary access on blocks.
>
> gdb/testsuite/ChangeLog:
>
> 2019-08-01 Christian Biesinger <cbiesinger@google.com>
>
> * gdb.python/py-block.exp: Test dictionary access on blocks.
OK for the documentation parts, thanks.
>>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:
Christian> 2019-08-01 Christian Biesinger <cbiesinger@google.com>
Christian> * NEWS: Mention dictionary access on blocks.
Christian> * python/py-block.c (blpy_getitem): New function.
Christian> (block_object_as_mapping): New struct.
Christian> (block_object_type): Use new struct for tp_as_mapping field.
Thank you for the patch.
Christian> + struct symbol *sym = block_lookup_symbol (
Christian> + block, name.get(), symbol_name_match_type::FULL, VAR_DOMAIN);
What about looking up things in other domains? I wonder if it's
possible to, say, have a local type that can be found by iteration over
the block but not by dictionary access. If so, that seems confusing.
Christian> + if (!sym) {
Christian> + PyErr_SetObject (PyExc_KeyError, key);
Christian> + return nullptr;
Christian> + }
The formatting here is incorrect. The call to block_lookup_symbol looks
weird (gdb doesn't generally use trailing parens like that), and the
brace after the "if" is misplaced.
Tom
On 2019-08-02 9:36 a.m., Tom Tromey wrote:
> Christian> + struct symbol *sym = block_lookup_symbol (
> Christian> + block, name.get(), symbol_name_match_type::FULL, VAR_DOMAIN);
>
> What about looking up things in other domains? I wonder if it's
> possible to, say, have a local type that can be found by iteration over
> the block but not by dictionary access. If so, that seems confusing.
Just wondering, was this commented responded to?
Simon
On Mon, Aug 5, 2019 at 1:17 PM Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-08-02 9:36 a.m., Tom Tromey wrote:
> > Christian> + struct symbol *sym = block_lookup_symbol (
> > Christian> + block, name.get(), symbol_name_match_type::FULL, VAR_DOMAIN);
> >
> > What about looking up things in other domains? I wonder if it's
> > possible to, say, have a local type that can be found by iteration over
> > the block but not by dictionary access. If so, that seems confusing.
>
> Just wondering, was this commented responded to?
Oh sorry, in the most recent version of the patch I fixed it by using
ALL_BLOCK_SYMBOLS_WITH_NAME, which should match the behavior of the
iterator afaict.
Christian
On 2019-08-05 14:19, Christian Biesinger wrote:
> On Mon, Aug 5, 2019 at 1:17 PM Simon Marchi <simark@simark.ca> wrote:
>>
>> On 2019-08-02 9:36 a.m., Tom Tromey wrote:
>> > Christian> + struct symbol *sym = block_lookup_symbol (
>> > Christian> + block, name.get(), symbol_name_match_type::FULL, VAR_DOMAIN);
>> >
>> > What about looking up things in other domains? I wonder if it's
>> > possible to, say, have a local type that can be found by iteration over
>> > the block but not by dictionary access. If so, that seems confusing.
>>
>> Just wondering, was this commented responded to?
>
> Oh sorry, in the most recent version of the patch I fixed it by using
> ALL_BLOCK_SYMBOLS_WITH_NAME, which should match the behavior of the
> iterator afaict.
Ok, thanks!
Note that it helps others (including the reviewer) if you acknowledge
the comments, either with Ok / Done or "I'd rather not do that because
X".
Simon
On Mon, Aug 5, 2019 at 1:28 PM Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-08-05 14:19, Christian Biesinger wrote:
> > On Mon, Aug 5, 2019 at 1:17 PM Simon Marchi <simark@simark.ca> wrote:
> >>
> >> On 2019-08-02 9:36 a.m., Tom Tromey wrote:
> >> > Christian> + struct symbol *sym = block_lookup_symbol (
> >> > Christian> + block, name.get(), symbol_name_match_type::FULL, VAR_DOMAIN);
> >> >
> >> > What about looking up things in other domains? I wonder if it's
> >> > possible to, say, have a local type that can be found by iteration over
> >> > the block but not by dictionary access. If so, that seems confusing.
> >>
> >> Just wondering, was this commented responded to?
> >
> > Oh sorry, in the most recent version of the patch I fixed it by using
> > ALL_BLOCK_SYMBOLS_WITH_NAME, which should match the behavior of the
> > iterator afaict.
>
> Ok, thanks!
>
> Note that it helps others (including the reviewer) if you acknowledge
> the comments, either with Ok / Done or "I'd rather not do that because
> X".
I did say at the top "Both issues should be fixed now." -- are you
suggesting it's better to individually acknowledge them? I'd be happy
to do that if y'all prefer.
Christian
@@ -47,6 +47,9 @@
** gdb.Objfile has new methods 'lookup_global_symbol' and
'lookup_static_symbol' to lookup a symbol from this objfile only.
+ ** gdb.Block now supports the dictionary syntax for accessing symbols in
+ this block (e.g. block['local_variable']).
+
* New commands
| [COMMAND] | SHELL_COMMAND
@@ -4722,7 +4722,12 @@ A @code{gdb.Block} is iterable. The iterator returns the symbols
should not assume that a specific block object will always contain a
given symbol, since changes in @value{GDBN} features and
infrastructure may cause symbols move across blocks in a symbol
-table.
+table. You can also use Python's @dfn{dictionary syntax} to access
+variables in this block, e.g.:
+
+@smallexample
+symbol = some_block['variable'] # symbol is of type gdb.Symbol
+@end smallexample
The following block-related functions are available in the @code{gdb}
module:
@@ -224,6 +224,30 @@ blpy_is_static (PyObject *self, void *closure)
Py_RETURN_FALSE;
}
+/* Given a string, returns the gdb.Symbol representing that symbol in this
+ block. If such a symbol does not exist, returns NULL with a Python
+ exception. */
+
+static PyObject *
+blpy_getitem (PyObject *self, PyObject *key)
+{
+ const struct block *block;
+
+ BLPY_REQUIRE_VALID (self, block);
+
+ gdb::unique_xmalloc_ptr<char> name = python_string_to_host_string (key);
+ if (name == nullptr)
+ return nullptr;
+
+ struct symbol *sym = block_lookup_symbol (
+ block, name.get(), symbol_name_match_type::FULL, VAR_DOMAIN);
+ if (!sym) {
+ PyErr_SetObject (PyExc_KeyError, key);
+ return nullptr;
+ }
+ return symbol_to_symbol_object (sym);
+}
+
static void
blpy_dealloc (PyObject *obj)
{
@@ -440,6 +464,12 @@ static gdb_PyGetSetDef block_object_getset[] = {
{ NULL } /* Sentinel */
};
+static PyMappingMethods block_object_as_mapping = {
+ NULL,
+ blpy_getitem,
+ NULL
+};
+
PyTypeObject block_object_type = {
PyVarObject_HEAD_INIT (NULL, 0)
"gdb.Block", /*tp_name*/
@@ -453,7 +483,7 @@ PyTypeObject block_object_type = {
0, /*tp_repr*/
0, /*tp_as_number*/
0, /*tp_as_sequence*/
- 0, /*tp_as_mapping*/
+ &block_object_as_mapping, /*tp_as_mapping*/
0, /*tp_hash */
0, /*tp_call*/
0, /*tp_str*/
@@ -43,6 +43,11 @@ gdb_test "python print (block)" "<gdb.Block object at $hex>" "check block not No
gdb_test "python print (block.function)" "None" "first anonymous block"
gdb_test "python print (block.start)" "${decimal}" "check start not None"
gdb_test "python print (block.end)" "${decimal}" "check end not None"
+gdb_test "python print (block\['f'\].name == 'f')" "True" "check variable access"
+gdb_test "python print (block\['nonexistent'\])" ".*KeyError: 'nonexistent'.*" \
+ "check nonexistent variable"
+gdb_test "python print (block\[42\])" ".*TypeError: Expected a string.*" \
+ "check non-string key"
# Test global/static blocks
gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0