gdb/python: Use tp_init instead of tp_new to setup gdb.Value

The documentation suggests that we implement gdb.Value.__init__,
however, this is not currently true, we really implement
gdb.Value.__new__.  This will cause confusion if a user tries to
sub-class gdb.Value.  They might write:

  class MyVal (gdb.Value):
      def __init__ (self, val):
          gdb.Value.__init__(self, val)

  obj = MyVal(123)
  print ("Got: %s" % obj)

But, when they source this code they'll see:

  (gdb) source ~/tmp/value-test.py
  Traceback (most recent call last):
    File "/home/andrew/tmp/value-test.py", line 7, in <module>
      obj = MyVal(123)
    File "/home/andrew/tmp/value-test.py", line 5, in __init__
      gdb.Value.__init__(self, val)
  TypeError: object.__init__() takes exactly one argument (the instance to initialize)
  (gdb)

The reason for this is that, as we don't implement __init__ for
gdb.Value, Python ends up calling object.__init__ instead, which
doesn't expect any arguments.

The Python docs suggest that the reason why we might take this
approach is because we want gdb.Value to be immutable:

   https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_new

But I don't see any reason why we should require gdb.Value to be
immutable when other types defined in GDB are not.  This current
immutability can be seen in this code:

  obj = gdb.Value(1234)
  print("Got: %s" % obj)
  obj.__init__ (5678)
  print("Got: %s" % obj)

Which currently runs without error, but prints:

  Got: 1234
  Got: 1234

In this commit I propose that we switch to using __init__ to
initialize gdb.Value objects.

This does introduce some additional complexity, during the __init__
call a gdb.Value might already be associated with a gdb value object,
in which case we need to cleanly break that association before
installing the new gdb value object.  However, the cost of doing this
is not great, and the benefit - being able to easily sub-class
gdb.Value seems worth it.

After this commit the first example above works without error, while
the second example now prints:

  Got: 1234
  Got: 5678

In order to make it easier to override the gdb.Value.__init__ method,
I have tweaked the definition of gdb.Value.__init__.  The second,
optional argument to __init__ is a gdb.Type, if this argument is not
present then GDB figures out a suitable type.

However, if we want to override the __init__ method in a sub-class,
and still support the default argument, it is easier to write:

  class MyVal (gdb.Value):
      def __init__ (self, val, type=None):
          gdb.Value.__init__(self, val, type)

Currently, passing None for the Type will result in an error:

  TypeError: type argument must be a gdb.Type.

After this commit I now allow the type argument to be None, in which
case GDB figures out a suitable type just as if the type had not been
passed at all.

Unless a user is trying to reinitialize a value, or create sub-classes
of gdb.Value, there should be no user visible changes after this
commit.

1 files changed, 54 insertions, 5 deletions

diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index f4b7c23..297b128 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -51,6 +51,7 @@ proc test_value_creation {} {
 
   gdb_py_test_silent_cmd "python i = gdb.Value (True)" "create boolean value" 1
   gdb_py_test_silent_cmd "python i = gdb.Value (5)" "create integer value" 1
+  gdb_py_test_silent_cmd "python i = gdb.Value (3,None)" "create integer value, with None type" 1
   if { $gdb_py_is_py3k == 0 } {
     gdb_py_test_silent_cmd "python i = gdb.Value (5L)" "create long value" 1
   }
@@ -77,6 +78,18 @@ proc test_value_creation {} {
   gdb_test "python print ('result = %s' % i.address)" "= None" "test address attribute in non-addressable value"
 }
 
+# Check that we can call gdb.Value.__init__ to change a value.
+proc test_value_reinit {} {
+    gdb_py_test_silent_cmd "python v = gdb.Value (3)" \
+	"create initial integer value" 1
+    gdb_test "python print(v)" "3" \
+	"check initial value contents"
+    gdb_py_test_silent_cmd "python v.__init__(5)" \
+	"call gdb.Value.__init__ manually" 1
+    gdb_test "python print(v)" "5" \
+	"check new value contents"
+}
+
 proc test_value_numeric_ops {} {
   global gdb_prompt
   global gdb_py_is_py3k
@@ -531,10 +544,14 @@ proc test_float_conversion {} {
     gdb_test "python print(float(gdb.Value(0)))" "0\\.0"
 }
 
-proc test_value_from_buffer {} {
-  global gdb_prompt
-  global gdb_py_is_py3k
-
+# Setup some Python variables:
+#   tp      : a gdb.Type for 'int',
+#   size_a  : the size of array 'a' from the inferior,
+#   size_a0 : the size of array element 'a[0] from the inferior,
+#   addr    : the address of 'a[0]' from the inferior,
+#   b       : a buffer containing the full contents of array 'a' from the
+#             inferior.
+proc prepare_type_and_buffer {} {
   gdb_py_test_silent_cmd "python tp=gdb.lookup_type('int')" "look up int type" 0
   gdb_py_test_silent_cmd "python size_a=gdb.parse_and_eval('sizeof(a)')" \
                          "find size of a" 0
@@ -543,7 +560,14 @@ proc test_value_from_buffer {} {
   gdb_py_test_silent_cmd "python addr=gdb.parse_and_eval('&a')" \
                          "find address of a" 0
   gdb_py_test_silent_cmd "python b=gdb.selected_inferior().read_memory(addr,size_a)" \
-                         "read buffer from memory" 1
+                         "read buffer from memory" 0
+}
+
+proc test_value_from_buffer {} {
+  global gdb_prompt
+  global gdb_py_is_py3k
+
+  prepare_type_and_buffer
   gdb_test "python v=gdb.Value(b,tp); print(v)" "1" \
             "construct value from buffer"
   gdb_test "python v=gdb.Value(b\[size_a0:\],tp); print(v)" "2" \
@@ -600,6 +624,29 @@ proc test_add_to_history {} {
 	"TypeError: Could not convert Python object: .*"
 }
 
+# Check we can create sub-classes of gdb.Value.
+proc test_value_sub_classes {} {
+    prepare_type_and_buffer
+
+    gdb_test_multiline "Create sub-class of gdb.Value" \
+	"python" "" \
+	"class MyValue(gdb.Value):" "" \
+	"  def __init__(self,val,type=None):" "" \
+	"    gdb.Value.__init__(self,val,type)" "" \
+	"    print(\"In MyValue.__init__\")" "" \
+	"end"
+
+    gdb_test "python obj = MyValue (123)" "In MyValue.__init__" \
+	"create instance of MyValue"
+    gdb_test "python print(obj)" "123" \
+	"check printing of MyValue"
+
+    gdb_test "python obj = MyValue(b\[size_a0:\],tp)" "In MyValue.__init__" \
+	"convert 2nd elem of buffer to a MyValue"
+    gdb_test "python print(obj)" "2" \
+	"check printing of MyValue when initiaized with a type"
+}
+
 # Build C version of executable.  C++ is built later.
 if { [build_inferior "${binfile}" "c"] < 0 } {
     return -1
@@ -612,6 +659,7 @@ clean_restart ${binfile}
 if { [skip_python_tests] } { continue }
 
 test_value_creation
+test_value_reinit
 test_value_numeric_ops
 test_value_boolean
 test_value_compare
@@ -629,6 +677,7 @@ if ![runto_main] then {
 
 test_value_in_inferior
 test_value_from_buffer
+test_value_sub_classes
 test_inferior_function_call
 test_value_after_death