RFA: fix PR python/11792

RFA: fix PR python/11792

[Tom Tromey]

This fixes PR python/11792, which is a feature request asking for a new
Value.dynamic_type method.  The PR includes a patch (Andre has an
assignment via his company), but I've tweaked it here and there and also
added docs and a test case.

This needs a doc review.

Built and regtested on x86-64 (compile farm).

Tom

2010-08-23  Andre Poenitz  <andre.poenitz@nokia.com>
	    Tom Tromey  <tromey@redhat.com>

	PR python/11792:
	* python/py-value.c (valpy_get_dynamic_type): New function.
	(value_object_getset): Add "dynamic_type".
	(valpy_get_type): Fail on error.

2010-08-23  Tom Tromey  <tromey@redhat.com>

	PR python/11792:
	* gdb.texinfo (Values From Inferior): Document dynamic_type.

2010-08-23  Tom Tromey  <tromey@redhat.com>

	PR python/11792:
	* gdb.python/py-value.exp (test_subscript_regression): Add
	dynamic_type test.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d7ff5aa..7e9c0c1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20737,6 +20737,16 @@ this value, thus it is not available for fetching from the inferior.
 The type of this @code{gdb.Value}.  The value of this attribute is a
 @code{gdb.Type} object.
 @end defivar
+
+@defivar Value dynamic_type
+The dynamic type of this @code{gdb.Value}.  This uses C@t{++} run-time
+type information to determine the dynamic type of the value.  If this
+value is of class type, it will return the class in which the value is
+embedded, if any.  If this value is of pointer or reference to a class
+type, it will compute the dynamic type of the referenced object, and
+return a pointer or reference to that type, respectively.  In all
+other cases, it will return the value's static type.
+@end defivar
 @end table
 
 The following methods are provided:
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index aa18042..0aeea7c 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -27,6 +27,7 @@
 #include "valprint.h"
 #include "infcall.h"
 #include "expression.h"
+#include "cp-abi.h"
 
 #ifdef HAVE_PYTHON
 
@@ -62,6 +63,7 @@ typedef struct value_object {
   struct value *value;
   PyObject *address;
   PyObject *type;
+  PyObject *dynamic_type;
 } value_object;
 
 /* List of all values which are currently exposed to Python. It is
@@ -101,6 +103,8 @@ valpy_dealloc (PyObject *obj)
       Py_DECREF (self->type);
     }
 
+  Py_XDECREF (self->dynamic_type);
+
   self->ob_type->tp_free (self);
 }
 
@@ -148,6 +152,7 @@ valpy_new (PyTypeObject *subtype, PyObject *args, PyObject *keywords)
   value_incref (value);
   value_obj->address = NULL;
   value_obj->type = NULL;
+  value_obj->dynamic_type = NULL;
   note_value (value_obj);
 
   return (PyObject *) value_obj;
@@ -218,15 +223,78 @@ valpy_get_type (PyObject *self, void *closure)
     {
       obj->type = type_to_type_object (value_type (obj->value));
       if (!obj->type)
-	{
-	  obj->type = Py_None;
-	  Py_INCREF (obj->type);
-	}
+	return NULL;
     }
   Py_INCREF (obj->type);
   return obj->type;
 }
 
+/* Return dynamic type of the value.  */
+
+static PyObject *
+valpy_get_dynamic_type (PyObject *self, void *closure)
+{
+  value_object *obj = (value_object *) self;
+  volatile struct gdb_exception except;
+  struct type *type = NULL;
+
+  if (obj->dynamic_type != NULL)
+    {
+      Py_INCREF (obj->dynamic_type);
+      return obj->dynamic_type;
+    }
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      struct value *val = obj->value;
+
+      type = value_type (val);
+      CHECK_TYPEDEF (type);
+
+      if (((TYPE_CODE (type) == TYPE_CODE_PTR)
+	   || (TYPE_CODE (type) == TYPE_CODE_REF))
+	  && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_CLASS))
+	{
+	  struct value *target;
+	  int was_pointer = TYPE_CODE (type) == TYPE_CODE_PTR;
+
+	  target = value_ind (val);
+	  type = value_rtti_type (target, NULL, NULL, NULL);
+
+	  if (type)
+	    {
+	      if (was_pointer)
+		type = lookup_pointer_type (type);
+	      else
+		type = lookup_reference_type (type);
+	    }
+	}
+      else if (TYPE_CODE (type) == TYPE_CODE_CLASS)
+	type = value_rtti_type (val, NULL, NULL, NULL);
+      else
+	{
+	  /* Re-use object's static type.  */
+	  type = NULL;
+	}
+    }
+  GDB_PY_HANDLE_EXCEPTION (except);
+
+  if (type == NULL)
+    {
+      /* Ensure that the TYPE field is ready.  */
+      if (!valpy_get_type (self, NULL))
+	return NULL;
+      /* We don't need to incref here, because valpy_get_type already
+	 did it for us.  */
+      obj->dynamic_type = obj->type;
+    }
+  else
+    obj->dynamic_type = type_to_type_object (type);
+
+  Py_INCREF (obj->dynamic_type);
+  return obj->dynamic_type;
+}
+
 /* Implementation of gdb.Value.lazy_string ([encoding] [, length]) ->
    string.  Return a PyObject representing a lazy_string_object type.
    A lazy string is a pointer to a string with an optional encoding and
@@ -994,6 +1062,7 @@ value_to_value_object (struct value *val)
       value_incref (val);
       val_obj->address = NULL;
       val_obj->type = NULL;
+      val_obj->dynamic_type = NULL;
       note_value (val_obj);
     }
 
@@ -1169,6 +1238,8 @@ static PyGetSetDef value_object_getset[] = {
     "Boolean telling whether the value is optimized out (i.e., not available).",
     NULL },
   { "type", valpy_get_type, NULL, "Type of the value.", NULL },
+  { "dynamic_type", valpy_get_dynamic_type, NULL,
+    "Dynamic type of the value.", NULL },
   {NULL}  /* Sentinel */
 };
 
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index 13bce9a..0ecb57c 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -379,6 +379,13 @@ proc test_subscript_regression {lang} {
      # the C++ tests.
      gdb_test "python print bool(gdb.parse_and_eval('base').dynamic_cast(gdb.lookup_type('Derived').pointer()))" \
 	 True
+
+     # Likewise.
+     gdb_test "python print gdb.parse_and_eval('base').dynamic_type" \
+	 "Derived \[*\]"
+     # A static type case.
+     gdb_test "python print gdb.parse_and_eval('5').dynamic_type" \
+	 "int"
  }
 
  gdb_breakpoint [gdb_get_line_number "break to inspect struct and union"]

Re: RFA: fix PR python/11792

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> This fixes PR python/11792, which is a feature request asking for a new
Tom> Value.dynamic_type method.  The PR includes a patch (Andre has an
Tom> assignment via his company), but I've tweaked it here and there and also
Tom> added docs and a test case.

Tom> This needs a doc review.

Ping.

Tom

Re: RFA: fix PR python/11792

[Eli Zaretskii]

> X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED,
> 	T_RP_MATCHES_RCVD autolearn=ham version=3.3.1
> From: Tom Tromey <tromey@redhat.com>
> Date: Mon, 30 Aug 2010 12:01:35 -0600
> 
> >>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
> 
> Tom> This fixes PR python/11792, which is a feature request asking for a new
> Tom> Value.dynamic_type method.  The PR includes a patch (Andre has an
> Tom> assignment via his company), but I've tweaked it here and there and also
> Tom> added docs and a test case.
> 
> Tom> This needs a doc review.
> 
> Ping.

Sorry.  The docs patch is okay, but I wonder whether we should say
something about what happens when the language is not C++.

Thanks.

Re: RFA: fix PR python/11792

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> Sorry.  The docs patch is okay, but I wonder whether we should say
Eli> something about what happens when the language is not C++.

The current language doesn't matter; all that matters is whether the
object has the needed run-time type information.

Tom

Re: RFA: fix PR python/11792

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Mon, 30 Aug 2010 14:24:16 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Eli> Sorry.  The docs patch is okay, but I wonder whether we should say
> Eli> something about what happens when the language is not C++.
> 
> The current language doesn't matter; all that matters is whether the
> object has the needed run-time type information.

Then IMO the text should be rephrased, because it currently sounds
like this feature needs C++.  Perhaps use C++ just "for example" or
something.

Re: RFA: fix PR python/11792

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Tom> The current language doesn't matter; all that matters is whether the
Tom> object has the needed run-time type information.

Eli> Then IMO the text should be rephrased, because it currently sounds
Eli> like this feature needs C++.  Perhaps use C++ just "for example" or
Eli> something.

I think that would be more confusing.  The information needed is
specifically that emitted by the C++ compiler, following the relevant
C++ ABI.  "For example" makes it sound as though other kinds of run-time
type information might be used -- but that is not the case.

I think the text is reasonably clear as it is now.

Tom

Re: RFA: fix PR python/11792

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 01 Sep 2010 16:37:34 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Tom> The current language doesn't matter; all that matters is whether the
> Tom> object has the needed run-time type information.
> 
> Eli> Then IMO the text should be rephrased, because it currently sounds
> Eli> like this feature needs C++.  Perhaps use C++ just "for example" or
> Eli> something.
> 
> I think that would be more confusing.  The information needed is
> specifically that emitted by the C++ compiler, following the relevant
> C++ ABI.  "For example" makes it sound as though other kinds of run-time
> type information might be used -- but that is not the case.

Now I'm confused: earlier you told that the current language doesn't
matter, but now you say that only information emitted by the C++
compiler will do, which seems a contradiction.  What am I missing?

Re: RFA: fix PR python/11792

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Tom> I think that would be more confusing.  The information needed is
Tom> specifically that emitted by the C++ compiler, following the relevant
Tom> C++ ABI.  "For example" makes it sound as though other kinds of run-time
Tom> type information might be used -- but that is not the case.

Eli> Now I'm confused: earlier you told that the current language doesn't
Eli> matter, but now you say that only information emitted by the C++
Eli> compiler will do, which seems a contradiction.  What am I missing?

The current language is a gdb setting.  It is independent of objects in
the inferior.

Tom

Re: RFA: fix PR python/11792

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Thu, 02 Sep 2010 09:36:57 -0600
> 
> Eli> Now I'm confused: earlier you told that the current language doesn't
> Eli> matter, but now you say that only information emitted by the C++
> Eli> compiler will do, which seems a contradiction.  What am I missing?
> 
> The current language is a gdb setting.  It is independent of objects in
> the inferior.

That's a misunderstanding, then: I didn't mean (and didn't say)
"current language".

So I understand now that this feature will work only while debugging a
C++ program that includes RTTI for the object in question.  Otherwise,
it will just return the type of the value as in "ptype foo", is that
right?  If so, maybe just mention this fact, since someone who debugs
a non-C++ program will not necessarily understand what is meant by
"the static type".

Thanks.

Re: RFA: fix PR python/11792

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

>> The current language is a gdb setting.  It is independent of objects in
>> the inferior.

Eli> That's a misunderstanding, then: I didn't mean (and didn't say)
Eli> "current language".

Ok.

Eli> So I understand now that this feature will work only while debugging a
Eli> C++ program that includes RTTI for the object in question.  Otherwise,
Eli> it will just return the type of the value as in "ptype foo", is that
Eli> right?

That is reasonably accurate.

Eli> If so, maybe just mention this fact, since someone who debugs
Eli> a non-C++ program will not necessarily understand what is meant by
Eli> "the static type".

I still think the text is reasonably clear, but if you want to change
it, that is fine by me.

Tom

Re: RFA: fix PR python/11792

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Tue, 07 Sep 2010 12:50:13 -0600
> 
> I still think the text is reasonably clear, but if you want to change
> it, that is fine by me.

Done.

Re: RFA: fix PR python/11792

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Tom> I still think the text is reasonably clear, but if you want to change
Tom> it, that is fine by me.

Eli> Done.

I didn't see a patch posted.

Tom

Re: RFA: fix PR python/11792

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 22 Sep 2010 10:33:23 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Tom> I still think the text is reasonably clear, but if you want to change
> Tom> it, that is fine by me.
> 
> Eli> Done.
> 
> I didn't see a patch posted.

I didn't intend to.

Re: RFA: fix PR python/11792

[Joel Brobecker]

> I didn't intend to.

But why? Aren't we supposed to post all patches that get checked in?

-- 
Joel

Re: RFA: fix PR python/11792

[Eli Zaretskii]

> Date: Wed, 22 Sep 2010 11:25:04 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Tom Tromey <tromey@redhat.com>, gdb-patches@sourceware.org
> 
> > I didn't intend to.
> 
> But why? Aren't we supposed to post all patches that get checked in?

Yes.  We are also supposed to humor the responsible maintainer when
she asks for some simple change as part of the review process.

Re: RFA: fix PR python/11792

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> Yes.  We are also supposed to humor the responsible maintainer when
Eli> she asks for some simple change as part of the review process.

That is no reason to ignore the agreed-upon rules.

In this case I did not read your message as a request.  I saw it as a
conditional suggestion, which I chose not to take.

Tom

Re: RFA: fix PR python/11792

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: Joel Brobecker <brobecker@adacore.com>, gdb-patches@sourceware.org
> Date: Wed, 22 Sep 2010 13:17:25 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Eli> Yes.  We are also supposed to humor the responsible maintainer when
> Eli> she asks for some simple change as part of the review process.
> 
> That is no reason to ignore the agreed-upon rules.

It is to me.  The agreed-upon rules are a two-way street, you know.
If people are not interested to see the manual fit the quality
standards that I consider important (which aren't much, if you follow
my reviews), I don't see why they should be interested in seeing my
patches to fix that what they declined to.  Not to mention the fact
that every commit is visible after you resync with the repository,
anyway.

> In this case I did not read your message as a request.  I saw it as a
> conditional suggestion, which I chose not to take.

I don't see any difference between a suggestion and a request, when it
comes from the responsible maintainer.  And I don't see a difference
between "chose not to take" and "decline to" (a.k.a. "refuse").  I'm
not going to fight with people to get my "suggestions" into the manual
against their explicitly expressed will (or lack thereof).

Re: RFA: fix PR python/11792

[Tom Tromey]

Eli> Yes.  We are also supposed to humor the responsible maintainer when
Eli> she asks for some simple change as part of the review process.

Tom> That is no reason to ignore the agreed-upon rules.

Eli> It is to me.  The agreed-upon rules are a two-way street, you know.

I don't see how your behavior helps the situation.

Several alternatives to this approach were available to you.  For
example, you could have sent email saying that it was not a suggestion.
Or you could have said that in the first place.  Or, you could even have
asked me to revert my patch.

Tom> In this case I did not read your message as a request.  I saw it as a
Tom> conditional suggestion, which I chose not to take.

Eli> I don't see any difference between a suggestion and a request, when it
Eli> comes from the responsible maintainer.

You once told me that you never vetoed a patch, and that "Disagreement,
even a strong one, is not a veto unless you perceive it as such".  That
is how I have read all email from you from then on -- I try to make the
changes you like, but in the end, I rely on my own judgment.

Eli> I'm not going to fight with people to get my "suggestions" into the
Eli> manual against their explicitly expressed will (or lack thereof).

Sounds good.  Just post your patches, like all other contributors.

Tom

Re: RFA: fix PR python/11792

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: brobecker@adacore.com, gdb-patches@sourceware.org
> Date: Wed, 22 Sep 2010 13:50:55 -0600
> 
> Eli> Yes.  We are also supposed to humor the responsible maintainer when
> Eli> she asks for some simple change as part of the review process.
> 
> Tom> That is no reason to ignore the agreed-upon rules.
> 
> Eli> It is to me.  The agreed-upon rules are a two-way street, you know.
> 
> I don't see how your behavior helps the situation.

It is an expression of protest in the most quiet way I could come up
with.  If you'd just let it go, it would have stayed that way.

> Several alternatives to this approach were available to you.  For
> example, you could have sent email saying that it was not a suggestion.
> Or you could have said that in the first place.  Or, you could even have
> asked me to revert my patch.

None of these alternatives seemed better than what I did, given the
"do it yourself, if you want" response I got for my comments.

> Tom> In this case I did not read your message as a request.  I saw it as a
> Tom> conditional suggestion, which I chose not to take.
> 
> Eli> I don't see any difference between a suggestion and a request, when it
> Eli> comes from the responsible maintainer.
> 
> You once told me that you never vetoed a patch, and that "Disagreement,
> even a strong one, is not a veto unless you perceive it as such".  That
> is how I have read all email from you from then on -- I try to make the
> changes you like, but in the end, I rely on my own judgment.

Sorry, but you are abusing the intentionally soft tone of my review
comments and my urge to find a compromise almost at any cost.  I don't
veto patches, but I don't expect blunt refusal to do anything at all
about some of my comments, either.  I respect your judgment, but the
final approval of the patch is still my responsibility.  Until I say
"OK" or something to that effect, the discussion is not over and the
patch cannot go in asking me to do the rest if I want to.  These are
the rules, as far as I understand them; if I missed something, please
point out what that is.