[RFC/Patch v3] PR 16113: Allow gdb.Field objects as subscripts on gdb.Value objects

[RFC/Patch v3] PR 16113: Allow gdb.Field objects as subscripts on gdb.Value objects

[Siva Chandra]

[-- Attachment #1: Type: text/plain, Size: 2546 bytes --]

The attached patch addresses all of Tom's comments except two for
which I have responses below:

Tom> What about TYPE_CODE_UNION and TYPE_CODE_RANGE?
Tom> Offhand I don't recall if other types present "fields".
Tom> Maybe TYPE_CODE_FUNC?  TYPE_CODE_COMPLEX?

I missed TYPE_CODE_UNION so I have added it in the attached patch. The
other types do not support the "fields" method. [But, is it a mistake
that we do not support fields for other methods? The documentation
does talk about ranges, functions and methods. At this point, I am
tempted to ask as to why we should "overload" the method "fields" to
return function/method args? Can we bring in a class hierarchy of
types where in gdb.Type exists as a base class, and gdb.TypeStruct,
gdb.TypeFunction etc. are derived from it. That way, we can add
relevant methods to the respective classes. For example,
gdb.TypeFunction will have "args" as a method and not "fields". I
understand the notion of fields is overloaded internally, but we could
make the Python API clearer.]

Siva> +  arg = type_to_type_object (type);
Siva> +  if (arg == NULL)
Siva> +    goto fail;

Tom> I think if TYPE is not a gdb.Type then this will return -1 without
Tom> setting the Python exception.

The above comment is for the change in py-type.c. The variable TYPE in
question here is "struct type" and not gdb.Type. I think
type_to_type_object would return NULL only if PyObject_New called in
it fails. Is an exception not set by PyObject_New on failure?

2013-12-11  Siva Chandra Reddy  <sivachandra@google.com>

        PR 16113: Allow gdb.Field objects with the subscript operator on
        gdb.Value objects of struct/class values.
        * NEWS (Python Scripting): Add entry for the new feature and the
        new attribute of gdb.Field objects.
        * python/py-type.c (gdbpy_is_field): New function
        (convert_field): Add 'parent_type' attribute to gdb.Field
        objects.
        * python/py-value.c (valpy_getitem): Allow subscript value to be
        a gdb.Field object.
        (value_has_field): New function
        (get_field_flag): New function
        * python/python-internal.h (gdbpy_is_field): Add declaration.

        testsuite/
        * gdb.python/py-value-cc.cc: Improve test case.
        * gdb.python/py-value-cc.exp: Add new tests.

        doc/
        * gdb.texinfo (Values From Inferior): Add a note about using
        gdb.Field objects as subscripts on gdb.Value objects.
        (Types In Python): Add description about the new attribute of
        gdb.Field objects.

[-- Attachment #2: value_field_subscript_patch_v3.txt --]
[-- Type: text/plain, Size: 11051 bytes --]

diff --git a/gdb/NEWS b/gdb/NEWS
index eff057f..e4baf50 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -40,6 +40,8 @@
   ** Frame filters and frame decorators have been added.
   ** Temporary breakpoints are now supported.
   ** Line tables representation has been added.
+  ** New attribute 'parent_type' for gdb.Field objects.
+  ** gdb.Field objects can be used as subscripts on gdb.Value objects.
 
 * New targets
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7dfa9f6..49dce03 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23985,7 +23985,17 @@ can access its @code{foo} element with:
 bar = some_val['foo']
 @end smallexample
 
-Again, @code{bar} will also be a @code{gdb.Value} object.
+@cindex getting structure elements using gdb.Field objects as subscripts
+Again, @code{bar} will also be a @code{gdb.Value} object.  Structure
+elements can also be accessed by using @code{gdb.Field} objects as
+subscripts (@xref{Types In Python} for more information on
+@code{gdb.Field} objects).  For example, if @code{foo_field} is a
+@code{gdb.Field} object corresponding to element @code{foo} of the above
+structure, then @code{bar} can also be accessed as follows:
+
+@smallexample
+bar = some_val[foo_field]
+@end smallexample
 
 A @code{gdb.Value} that represents a function can be executed via
 inferior function call.  Any arguments provided to the call must match
@@ -24367,6 +24377,10 @@ this will be zero; in this case the field's size is given by its type.
 @item type
 The type of the field.  This is usually an instance of @code{Type},
 but it can be @code{None} in some situations.
+
+@item parent_type
+The type which contains this field.  This is an instance of
+@code{gdb.Type}.
 @end table
 @end defun
 
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 9aeb92c..4ad1e12 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -146,6 +146,14 @@ field_new (void)
 
 \f
 
+/* Return true if OBJ is of type gdb.Field, false otherwise.  */
+
+int
+gdbpy_is_field (PyObject *obj)
+{
+  return PyObject_TypeCheck (obj, &field_object_type);
+}
+
 /* Return the code for this type.  */
 static PyObject *
 typy_get_code (PyObject *self, void *closure)
@@ -167,6 +175,13 @@ convert_field (struct type *type, int field)
   if (!result)
     return NULL;
 
+  arg = type_to_type_object (type);
+  if (arg == NULL)
+    goto fail;
+  if (PyObject_SetAttrString (result, "parent_type", arg) < 0)
+    goto failarg;
+  Py_DECREF (arg);
+
   if (!field_is_static (&TYPE_FIELD (type, field)))
     {
       const char *attrstring;
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 40254b9..df25179 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -499,13 +499,80 @@ valpy_length (PyObject *self)
   return -1;
 }
 
-/* Given string name of an element inside structure, return its value
-   object.  Returns NULL on error, with a python exception set.  */
+/* Return 1 if the gdb.Field object FIELD is present in the value V.
+   Returns 0 otherwise.  If any Python error occurs, -1 is returned.  */
+
+static int
+value_has_field (struct value *v, PyObject *field)
+{
+  struct type *parent_type, *val_type;
+  enum type_code type_code;
+  PyObject *type_object = PyObject_GetAttrString (field, "parent_type");
+  volatile struct gdb_exception except;
+  int has_field = 0;
+
+  if (type_object == NULL)
+    return -1;
+
+  parent_type = type_object_to_type (type_object);
+  Py_DECREF (type_object);
+  if (parent_type == NULL)
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       _("'parent_type' attribute of gdb.Field object is not a"
+			 "gdb.Type object."));
+      return -1;
+    }
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      val_type = value_type (v);
+      val_type = check_typedef (val_type);
+      if (TYPE_CODE (val_type) == TYPE_CODE_REF
+	  || TYPE_CODE (val_type) == TYPE_CODE_PTR)
+      val_type = check_typedef (TYPE_TARGET_TYPE (val_type));
+
+      type_code = TYPE_CODE (val_type);
+      if ((type_code == TYPE_CODE_STRUCT || type_code == TYPE_CODE_UNION)
+	  && types_equal (val_type, parent_type))
+	has_field = 1;
+      else
+	has_field = 0;
+    }
+  GDB_PY_SET_HANDLE_EXCEPTION (except);
+
+  return has_field;
+}
+
+/* Return the value of a flag FLAG_NAME in a gdb.Field object FIELD.
+   Returns 1 if the flag value is true, 0 if it is false, and -1 if
+   a Python error occurs.  */
+
+static int
+get_field_flag (PyObject *field, const char *flag_name)
+{
+  int flag_value;
+  PyObject *flag_object = PyObject_GetAttrString (field, flag_name);
+
+  if (flag_object == NULL)
+    return -1;
+
+  flag_value = PyObject_IsTrue (flag_object);
+  Py_DECREF (flag_object);
+
+  return flag_value;
+}
+
+/* Given string name or a gdb.Field object corresponding to an element inside
+   a structure, return its value object.  Returns NULL on error, with a python
+   exception set.  */
+
 static PyObject *
 valpy_getitem (PyObject *self, PyObject *key)
 {
   value_object *self_value = (value_object *) self;
   char *field = NULL;
+  PyObject *base_class_type_object = NULL;
   volatile struct gdb_exception except;
   PyObject *result = NULL;
 
@@ -515,6 +582,44 @@ valpy_getitem (PyObject *self, PyObject *key)
       if (field == NULL)
 	return NULL;
     }
+  else if (gdbpy_is_field (key))
+    {
+      int is_base_class, valid_field;
+
+      valid_field = value_has_field (self_value->value, key);
+      if (valid_field < 0)
+	return NULL;
+      else if (valid_field == 0)
+	{
+	  PyErr_SetString (PyExc_TypeError,
+			   _("Invalid lookup for a field not contained in "
+			     "the value."));
+
+	  return NULL;
+	}
+
+      is_base_class = get_field_flag (key, "is_base_class");
+      if (is_base_class < 0)
+	return NULL;
+      else if (is_base_class > 0)
+	{
+	  base_class_type_object = PyObject_GetAttrString (key, "type");
+	  if (base_class_type_object == NULL)
+	    return NULL;
+	}
+      else
+	{
+	  PyObject *name_obj = PyObject_GetAttrString (key, "name");
+
+	  if (name_obj == NULL)
+	    return NULL;
+
+	  field = python_string_to_host_string (name_obj);
+	  Py_DECREF (name_obj);
+	  if (field == NULL)
+	    return NULL;
+	}
+    }
 
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
@@ -524,6 +629,23 @@ valpy_getitem (PyObject *self, PyObject *key)
 
       if (field)
 	res_val = value_struct_elt (&tmp, NULL, field, 0, NULL);
+      else if (base_class_type_object != NULL)
+	{
+	  struct type *base_class_type, *val_type;
+
+	  base_class_type = type_object_to_type (base_class_type_object);
+	  Py_DECREF (base_class_type_object);
+	  if (base_class_type == NULL)
+	    error (_("Field type not an instance of gdb.Type."));
+
+	  val_type = check_typedef (value_type (tmp));
+	  if (TYPE_CODE (val_type) == TYPE_CODE_PTR)
+	    res_val = value_cast (lookup_pointer_type (base_class_type), tmp);
+	  else if (TYPE_CODE (val_type) == TYPE_CODE_REF)
+	    res_val = value_cast (lookup_reference_type (base_class_type), tmp);
+	  else
+	    res_val = value_cast (base_class_type, tmp);
+	}
       else
 	{
 	  /* Assume we are attempting an array access, and let the
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 125670e..dade7c7 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -290,6 +290,7 @@ PyObject *gdbpy_newest_frame (PyObject *self, PyObject *args);
 PyObject *gdbpy_selected_frame (PyObject *self, PyObject *args);
 PyObject *gdbpy_block_for_pc (PyObject *self, PyObject *args);
 PyObject *gdbpy_lookup_type (PyObject *self, PyObject *args, PyObject *kw);
+int gdbpy_is_field (PyObject *obj);
 PyObject *gdbpy_create_lazy_string_object (CORE_ADDR address, long length,
 					   const char *encoding,
 					   struct type *type);
diff --git a/gdb/testsuite/gdb.python/py-value-cc.cc b/gdb/testsuite/gdb.python/py-value-cc.cc
index c010fc9..80094ec 100644
--- a/gdb/testsuite/gdb.python/py-value-cc.cc
+++ b/gdb/testsuite/gdb.python/py-value-cc.cc
@@ -16,8 +16,21 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 class A {
+ public:
+  int a;
 };
 
+union U {
+  int a;
+  char c;
+};
+
+class B : public A {
+ public:
+  char a;
+};
+
+typedef B Btd;
 typedef int *int_ptr;
 
 int
@@ -28,6 +41,22 @@ func (const A &a)
   int_ptr ptr = &val;
   int_ptr &int_ptr_ref = ptr;
 
+  B b;
+  B b1;
+
+  b.a = 'a';
+  b.A::a = 10;
+
+  B *b_obj = &b1;
+  b_obj->a = 'b';
+  b_obj->A::a = 100;
+
+  B &b_ref = b1;
+  Btd &b_td = b1;
+
+  U u;
+  u.a = 99;
+
   return 0; /* Break here.  */
 }
 
@@ -35,5 +64,6 @@ int
 main ()
 {
   A obj;
+
   return func (obj);
 }
diff --git a/gdb/testsuite/gdb.python/py-value-cc.exp b/gdb/testsuite/gdb.python/py-value-cc.exp
index 55c3b97..eacaf2e 100644
--- a/gdb/testsuite/gdb.python/py-value-cc.exp
+++ b/gdb/testsuite/gdb.python/py-value-cc.exp
@@ -44,3 +44,38 @@ gdb_test "python print (str(gdb.parse_and_eval(\"int_ptr_ref\").dereference().ty
 gdb_test "python print (str(gdb.parse_and_eval(\"int_ptr_ref\").referenced_value().type))" "int_ptr"
 gdb_test "python print (str(gdb.parse_and_eval(\"int_ptr_ref\").referenced_value().dereference()))" "10"
 gdb_test "python print (str(gdb.parse_and_eval(\"int_ptr_ref\").referenced_value().referenced_value()))" "10"
+
+# Tests for gdb.Value[gdb.Field]
+gdb_test_no_output "python b = gdb.parse_and_eval('b')" "init b"
+gdb_test_no_output "python b_fields = b.type.fields()" "init b_fields"
+gdb_test_no_output "python b_obj = gdb.parse_and_eval('b_obj')" "init b_obj"
+gdb_test_no_output "python b_ref = gdb.parse_and_eval('b_ref')" "init b_ref"
+gdb_test_no_output "python b_td = gdb.parse_and_eval('b_td')" "init b_td"
+gdb_test_no_output "python u = gdb.parse_and_eval('u')" "init u"
+gdb_test_no_output "python u_fields = u.type.fields()" "init u_fields"
+
+gdb_test "python print(b\[b_fields\[1\]\])" "97 'a'" "b.a via field"
+gdb_test "python print(b\[b_fields\[0\]\].type)" "A" \
+  "type of b's base class via field"
+gdb_test "python print(b\[b_fields\[0\]\]\['a'\])" "10" "b.A::a via field"
+
+gdb_test "python print(b_obj\[b_fields\[1\]\])" "98 'b'" "b_obj->a via field"
+gdb_test "python print(b_obj\[b_fields\[0\]\].type.target())" "A" \
+  "type of b_obj's base class via field"
+gdb_test "python print(b_obj\[b_fields\[0\]\]\['a'\])" "100" \
+  "b_obj->A::a via field"
+
+gdb_test "python print(b_ref\[b_fields\[1\]\])" "98 'b'" "b_ref.a via field"
+gdb_test "python print(b_ref\[b_fields\[0\]\].type.target())" "A" \
+  "type of b_ref's base class via field"
+gdb_test "python print(b_ref\[b_fields\[0\]\]\['a'\])" "100" \
+  "b_ref.A::a via field"
+
+gdb_test "python print(b_td\[b_fields\[1\]\])" "98 'b'" "b_td.a via field"
+gdb_test "python print(b_td\[b_fields\[0\]\].type.target())" "A" \
+  "type of b_td's base class via field"
+gdb_test "python print(b_td\[b_fields\[0\]\]\['a'\])" "100" \
+  "b_td.A::a via field"
+
+gdb_test "python print(u\[u_fields\[0\]\])" "99.*" "u's first field via field"
+gdb_test "python print(u\[u_fields\[1\]\])" "99.*" "u's second field via field"

Re: [RFC/Patch v3] PR 16113: Allow gdb.Field objects as subscripts on gdb.Value objects

[Tom Tromey]

Tom> What about TYPE_CODE_UNION and TYPE_CODE_RANGE?
Tom> Offhand I don't recall if other types present "fields".
Tom> Maybe TYPE_CODE_FUNC?  TYPE_CODE_COMPLEX?

Siva> I missed TYPE_CODE_UNION so I have added it in the attached patch. The
Siva> other types do not support the "fields" method. [But, is it a mistake
Siva> that we do not support fields for other methods? The documentation
Siva> does talk about ranges, functions and methods. At this point, I am
Siva> tempted to ask as to why we should "overload" the method "fields" to
Siva> return function/method args? Can we bring in a class hierarchy of
Siva> types where in gdb.Type exists as a base class, and gdb.TypeStruct,
Siva> gdb.TypeFunction etc. are derived from it. That way, we can add
Siva> relevant methods to the respective classes. For example,
Siva> gdb.TypeFunction will have "args" as a method and not "fields". I
Siva> understand the notion of fields is overloaded internally, but we could
Siva> make the Python API clearer.]

Yeah, I agree, that would be cleaner.
Occasionally it's easy to become blind to the issue after working on gdb
for a while.

Siva> +  arg = type_to_type_object (type);
Siva> +  if (arg == NULL)
Siva> +    goto fail;

Tom> I think if TYPE is not a gdb.Type then this will return -1 without
Tom> setting the Python exception.

Siva> The above comment is for the change in py-type.c. The variable TYPE in
Siva> question here is "struct type" and not gdb.Type. I think
Siva> type_to_type_object would return NULL only if PyObject_New called in
Siva> it fails. Is an exception not set by PyObject_New on failure?

This one is my mistake.  I misread that as type_object_to_type.

Siva> 2013-12-11  Siva Chandra Reddy  <sivachandra@google.com>

Siva>         PR 16113: Allow gdb.Field objects with the subscript operator on

I don't remember if the regexp catches this form, best to write
"PR python/16113" instead.

The patch ok with this change, thanks!

Tom

Re: [RFC/Patch v3] PR 16113: Allow gdb.Field objects as subscripts on gdb.Value objects

[Siva Chandra]

This requires a doc review as there is small addition to NEWS and
gdb.texinfo after Eli approved the changes last.

On Wed, Dec 11, 2013 at 3:23 PM, Siva Chandra <sivachandra@google.com> wrote:
> The attached patch addresses all of Tom's comments except two for
> which I have responses below:
>
> Tom> What about TYPE_CODE_UNION and TYPE_CODE_RANGE?
> Tom> Offhand I don't recall if other types present "fields".
> Tom> Maybe TYPE_CODE_FUNC?  TYPE_CODE_COMPLEX?
>
> I missed TYPE_CODE_UNION so I have added it in the attached patch. The
> other types do not support the "fields" method. [But, is it a mistake
> that we do not support fields for other methods? The documentation
> does talk about ranges, functions and methods. At this point, I am
> tempted to ask as to why we should "overload" the method "fields" to
> return function/method args? Can we bring in a class hierarchy of
> types where in gdb.Type exists as a base class, and gdb.TypeStruct,
> gdb.TypeFunction etc. are derived from it. That way, we can add
> relevant methods to the respective classes. For example,
> gdb.TypeFunction will have "args" as a method and not "fields". I
> understand the notion of fields is overloaded internally, but we could
> make the Python API clearer.]
>
> Siva> +  arg = type_to_type_object (type);
> Siva> +  if (arg == NULL)
> Siva> +    goto fail;
>
> Tom> I think if TYPE is not a gdb.Type then this will return -1 without
> Tom> setting the Python exception.
>
> The above comment is for the change in py-type.c. The variable TYPE in
> question here is "struct type" and not gdb.Type. I think
> type_to_type_object would return NULL only if PyObject_New called in
> it fails. Is an exception not set by PyObject_New on failure?
>
> 2013-12-11  Siva Chandra Reddy  <sivachandra@google.com>
>
>         PR 16113: Allow gdb.Field objects with the subscript operator on
>         gdb.Value objects of struct/class values.
>         * NEWS (Python Scripting): Add entry for the new feature and the
>         new attribute of gdb.Field objects.
>         * python/py-type.c (gdbpy_is_field): New function
>         (convert_field): Add 'parent_type' attribute to gdb.Field
>         objects.
>         * python/py-value.c (valpy_getitem): Allow subscript value to be
>         a gdb.Field object.
>         (value_has_field): New function
>         (get_field_flag): New function
>         * python/python-internal.h (gdbpy_is_field): Add declaration.
>
>         testsuite/
>         * gdb.python/py-value-cc.cc: Improve test case.
>         * gdb.python/py-value-cc.exp: Add new tests.
>
>         doc/
>         * gdb.texinfo (Values From Inferior): Add a note about using
>         gdb.Field objects as subscripts on gdb.Value objects.
>         (Types In Python): Add description about the new attribute of
>         gdb.Field objects.

Re: [RFC/Patch v3] PR 16113: Allow gdb.Field objects as subscripts on gdb.Value objects

[Eli Zaretskii]

> Date: Thu, 12 Dec 2013 11:36:01 -0800
> From: Siva Chandra <sivachandra@google.com>
> Cc: Eli Zaretskii <eliz@gnu.org>
> 
> This requires a doc review as there is small addition to NEWS and
> gdb.texinfo after Eli approved the changes last.

Done.

Re: [RFC/Patch v3] PR 16113: Allow gdb.Field objects as subscripts on gdb.Value objects

[Eli Zaretskii]

> Date: Wed, 11 Dec 2013 15:23:36 -0800
> From: Siva Chandra <sivachandra@google.com>
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -40,6 +40,8 @@
>    ** Frame filters and frame decorators have been added.
>    ** Temporary breakpoints are now supported.
>    ** Line tables representation has been added.
> +  ** New attribute 'parent_type' for gdb.Field objects.
> +  ** gdb.Field objects can be used as subscripts on gdb.Value objects.

OK.

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -23985,7 +23985,17 @@ can access its @code{foo} element with:
>  bar = some_val['foo']
>  @end smallexample
>  
> -Again, @code{bar} will also be a @code{gdb.Value} object.
> +@cindex getting structure elements using gdb.Field objects as subscripts
> +Again, @code{bar} will also be a @code{gdb.Value} object.  Structure
> +elements can also be accessed by using @code{gdb.Field} objects as
> +subscripts (@xref{Types In Python} for more information on
                                     ^
Please use @pxref here, and add a comma after the closing brace, as
indicated.  Cross-references should have a comma, a period, or (in the
case of @pxref) a closing parenthesis after their closing brace.

OK with that change.

Thanks.