[rfa] misc fixes and improvements to Python code.

[rfa] misc fixes and improvements to Python code.

[Thiago Jung Bauermann]

Hi,

This patch brings some fixes and improvements from the Python branch to
code which was already committed in HEAD. The descriptions in the
ChangeLog are good enough. I'll only note that "keyword arguments" means
calling a function specifying an optional argument by it's name, as in
"gdb.some_function (foo, bar=baz)".

Ok?

Ah, I'll commit the changes in python-function.exp when the convenience
function patch gets committed...
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


gdb/
2009-03-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Jim Blandy  <jimb@red-bean.com>
	    Thiago Jung Bauermann  <bauerman@br.ibm.com>
	    Tom Tromey  <tromey@redhat.com>

	* python/python-cmd.c (cmdpy_init): Accept keyword
	arguments.
	* python/python-value.c (valpy_string): Accept keyword
	arguments.
	(valpy_binop): Use `break' to exit from the TRY_CATCH block.
	Do not call value_to_value_object on NULL RES_VAL.
	(value_object_methods): Change `string' entry to also accept
	keyword arguments.
	(convert_value_from_python): Return a copy of the value if obj is
	a gdb.Value object.
	(value_object_methods): Mark the `string' method as accepting
	keywords, and show method "prototype" in the doc string.
	* python/python.c (get_parameter): Don't return inside a
	TRY_CATCH.

gdb/doc/
2009-03-15  Thiago Jung Bauermann  <bauerman@br.ibm.com>
	* gdb.texinfo (Values From Inferior): Fix optional arguments
	markup.
	(Commands In Python): Adjust argument names of gdb.Command.__init__
	to what the function accepts as keywords.

gdb/testsuite/
2009-03-15  Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* gdb.python/python-cmd.exp: Add tests for keyword arguments.
	* gdb.python/python-function.exp: Add test for function returning
	a GDB value.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 286a676..bec2582 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18311,7 +18311,7 @@ The result @code{bar} will be a @code{gdb.Value} object holding the
 value pointed to by @code{foo}.
 @end defmethod
 
-@defmethod Value string @r{[}encoding @r{[}errors@r{]}@r{]}
+@defmethod Value string @r{[}encoding@r{]} @r{[}errors@r{]}
 If this @code{gdb.Value} represents a string, then this method
 converts the contents to a Python string.  Otherwise, this method will
 throw an exception.
@@ -18349,7 +18349,7 @@ You can implement new @value{GDBN} CLI commands in Python.  A CLI
 command is implemented using an instance of the @code{gdb.Command}
 class, most commonly using a subclass.
 
-@defmethod Command __init__ name @var{command-class} @r{[}@var{completer-class} @var{prefix}@r{]}
+@defmethod Command __init__ name @var{command_class} @r{[}@var{completer_class}@r{]} @r{[}@var{prefix}@r{]}
 The object initializer for @code{Command} registers the new command
 with @value{GDBN}.  This initializer is normally invoked from the
 subclass' own @code{__init__} method.
@@ -18361,11 +18361,11 @@ an exception is raised.
 
 There is no support for multi-line commands.
 
-@var{command-class} should be one of the @samp{COMMAND_} constants
+@var{command_class} should be one of the @samp{COMMAND_} constants
 defined below.  This argument tells @value{GDBN} how to categorize the
 new command in the help system.
 
-@var{completer-class} is an optional argument.  If given, it should be
+@var{completer_class} is an optional argument.  If given, it should be
 one of the @samp{COMPLETE_} constants defined below.  This argument
 tells @value{GDBN} how to perform completion for this command.  If not
 given, @value{GDBN} will attempt to complete using the object's
diff --git a/gdb/python/python-cmd.c b/gdb/python/python-cmd.c
index 36cde34..8f59a22 100644
--- a/gdb/python/python-cmd.c
+++ b/gdb/python/python-cmd.c
@@ -336,16 +336,16 @@ parse_command_name (char *text, struct cmd_list_element ***base_list)
 
 /* Object initializer; sets up gdb-side structures for command.
 
-   Use: __init__(NAME, CMDCLASS, [COMPLETERCLASS, [PREFIX]]).
+   Use: __init__(NAME, COMMAND_CLASS [, COMPLETER_CLASS][, PREFIX]]).
 
    NAME is the name of the command.  It may consist of multiple words,
    in which case the final word is the name of the new command, and
    earlier words must be prefix commands.
 
-   CMDCLASS is the kind of command.  It should be one of the COMMAND_*
+   COMMAND_CLASS is the kind of command.  It should be one of the COMMAND_*
    constants defined in the gdb module.
 
-   COMPLETERCLASS is the kind of completer.  If not given, the
+   COMPLETER_CLASS is the kind of completer.  If not given, the
    "complete" method will be used.  Otherwise, it should be one of the
    COMPLETE_* constants defined in the gdb module.
 
@@ -356,7 +356,7 @@ parse_command_name (char *text, struct cmd_list_element ***base_list)
    
 */
 static int
-cmdpy_init (PyObject *self, PyObject *args, PyObject *kwds)
+cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
 {
   cmdpy_object *obj = (cmdpy_object *) self;
   char *name;
@@ -366,6 +366,8 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kwds)
   volatile struct gdb_exception except;
   struct cmd_list_element **cmd_list;
   char *cmd_name, *pfx_name;
+  static char *keywords[] = { "name", "command_class", "completer_class",
+			      "prefix", NULL };
   PyObject *is_prefix = NULL;
   int cmp;
 
@@ -378,7 +380,7 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kwds)
       return -1;
     }
 
-  if (! PyArg_ParseTuple (args, "si|iO", &name, &cmdtype,
+  if (! PyArg_ParseTupleAndKeywords (args, kw, "si|iO", keywords, &name, &cmdtype,
 			  &completetype, &is_prefix))
     return -1;
 
diff --git a/gdb/python/python-value.c b/gdb/python/python-value.c
index bc077b6..cbc481f 100644
--- a/gdb/python/python-value.c
+++ b/gdb/python/python-value.c
@@ -143,10 +143,11 @@ valpy_address (PyObject *self, PyObject *args)
   return value_to_value_object (res_val);
 }
 
-/* Return Unicode string with value contents (assumed to be encoded in the
-   target's charset).  */
+/* Implementation of gdb.Value.string ([encoding] [, errors]) -> string
+   Return Unicode string with value contents.  If ENCODING is not given,
+   the string is assumed to be encoded in the target's charset.  */
 static PyObject *
-valpy_string (PyObject *self, PyObject *args)
+valpy_string (PyObject *self, PyObject *args, PyObject *kw)
 {
   int length, ret = 0;
   gdb_byte *buffer;
@@ -157,8 +158,10 @@ valpy_string (PyObject *self, PyObject *args)
   const char *errors = NULL;
   const char *user_encoding = NULL;
   const char *la_encoding = NULL;
+  static char *keywords[] = { "encoding", "errors" };
 
-  if (!PyArg_ParseTuple (args, "|ss", &user_encoding, &errors))
+  if (!PyArg_ParseTupleAndKeywords (args, kw, "|ss", keywords,
+				    &user_encoding, &errors))
     return NULL;
 
   TRY_CATCH (except, RETURN_MASK_ALL)
@@ -306,11 +309,11 @@ valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other)
 	 a gdb.Value object and need to convert it from python as well.  */
       arg1 = convert_value_from_python (self);
       if (arg1 == NULL)
-	return NULL;
+	break;
 
       arg2 = convert_value_from_python (other);
       if (arg2 == NULL)
-	return NULL;
+	break;
 
       switch (opcode)
 	{
@@ -387,7 +390,7 @@ valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other)
     }
   GDB_PY_HANDLE_EXCEPTION (except);
 
-  return value_to_value_object (res_val);
+  return res_val ? value_to_value_object (res_val) : NULL;
 }
 
 static PyObject *
@@ -774,7 +777,7 @@ convert_value_from_python (PyObject *obj)
 	    }
 	}
       else if (PyObject_TypeCheck (obj, &value_object_type))
-	value = ((value_object *) obj)->value;
+	value = value_copy (((value_object *) obj)->value);
       else
 	PyErr_Format (PyExc_TypeError, _("Could not convert Python object: %s"),
 		      PyString_AsString (PyObject_Str (obj)));
@@ -825,8 +828,9 @@ gdbpy_initialize_values (void)
 static PyMethodDef value_object_methods[] = {
   { "address", valpy_address, METH_NOARGS, "Return the address of the value." },
   { "dereference", valpy_dereference, METH_NOARGS, "Dereferences the value." },
-  { "string", valpy_string, METH_VARARGS,
-    "Return Unicode string representation of the value." },
+  { "string", (PyCFunction) valpy_string, METH_VARARGS | METH_KEYWORDS,
+    "string ([encoding] [, errors]) -> string\n\
+Return Unicode string representation of the value." },
   {NULL}  /* Sentinel */
 };
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 02d97e3..52fc780 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -206,6 +206,7 @@ get_parameter (PyObject *self, PyObject *args)
 {
   struct cmd_list_element *alias, *prefix, *cmd;
   char *arg, *newarg;
+  int found = -1;
   volatile struct gdb_exception except;
 
   if (! PyArg_ParseTuple (args, "s", &arg))
@@ -215,15 +216,13 @@ get_parameter (PyObject *self, PyObject *args)
 
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
-      if (! lookup_cmd_composition (newarg, &alias, &prefix, &cmd))
-	{
-	  xfree (newarg);
-	  return PyErr_Format (PyExc_RuntimeError,
-			       "could not find variable `%s'", arg);
-	}
+      found = lookup_cmd_composition (newarg, &alias, &prefix, &cmd);
     }
   xfree (newarg);
   GDB_PY_HANDLE_EXCEPTION (except);
+  if (!found)
+    return PyErr_Format (PyExc_RuntimeError,
+			 "could not find parameter `%s'", arg);
 
   if (! cmd->var)
     return PyErr_Format (PyExc_RuntimeError, "`%s' is not a variable", arg);
diff --git a/gdb/testsuite/gdb.python/python-cmd.exp b/gdb/testsuite/gdb.python/python-cmd.exp
index 6c73ff2..f6ef938 100644
--- a/gdb/testsuite/gdb.python/python-cmd.exp
+++ b/gdb/testsuite/gdb.python/python-cmd.exp
@@ -92,6 +92,32 @@ gdb_py_test_multiple "input subcommand" \
 
 gdb_test "prefix_cmd subcmd ugh" "subcmd output, arg = ugh" "call subcmd"
 
+# Test prefix command using keyword arguments.
+
+gdb_py_test_multiple "input prefix command, keyword arguments" \
+  "python" "" \
+  "class prefix_cmd2 (gdb.Command):" "" \
+  "  def __init__ (self):" "" \
+  "    super (prefix_cmd2, self).__init__ (\"prefix_cmd2\", gdb.COMMAND_OBSCURE, prefix = True, completer_class = gdb.COMPLETE_FILENAME)" "" \
+  "  def invoke (self, arg, from_tty):" "" \
+  "    print \"prefix_cmd2 output, arg = %s\" % arg" "" \
+  "prefix_cmd2 ()" "" \
+  "end" ""
+
+gdb_test "prefix_cmd2 argh" "prefix_cmd2 output, arg = argh" "call prefix command, keyword arguments"
+
+gdb_py_test_multiple "input subcommand under prefix_cmd2" \
+  "python" "" \
+  "class subcmd (gdb.Command):" "" \
+  "  def __init__ (self):" "" \
+  "    super (subcmd, self).__init__ (\"prefix_cmd2 subcmd\", gdb.COMMAND_OBSCURE)" "" \
+  "  def invoke (self, arg, from_tty):" "" \
+  "    print \"subcmd output, arg = %s\" % arg" "" \
+  "subcmd ()" "" \
+  "end" ""
+
+gdb_test "prefix_cmd2 subcmd ugh" "subcmd output, arg = ugh" "call subcmd under prefix_cmd2"
+
 # Test a subcommand in an existing GDB prefix.
 
 gdb_py_test_multiple "input new subcommand" \
diff --git a/gdb/testsuite/gdb.python/python-function.exp b/gdb/testsuite/gdb.python/python-function.exp
index 3ffc5aa..7feca2b 100644
--- a/gdb/testsuite/gdb.python/python-function.exp
+++ b/gdb/testsuite/gdb.python/python-function.exp
@@ -63,3 +63,17 @@ gdb_py_test_multiple "input convenience function" \
   "end" ""
 
 gdb_test "print \$test_func (\"ugh\")" "= \"test_func output, arg = ugh\"" "call function"
+
+# Test returning a gdb.Value from the function. This segfaulted GDB at one point.
+
+gdb_py_test_multiple "input value-returning convenience function" \
+  "python" "" \
+  "class Double (gdb.Function):" "" \
+  "  def __init__ (self):" "" \
+  "    super (Double, self).__init__ (\"double\")" "" \
+  "  def invoke (self, n):" "" \
+  "    return n*2" "" \
+  "Double ()" "" \
+  "end" ""
+
+gdb_test "print \$double (1)" "= 2" "call value-returning function"

Re: [rfa] misc fixes and improvements to Python code.

[Tom Tromey]

>>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:

Thiago> This patch brings some fixes and improvements from the Python
Thiago> branch to code which was already committed in HEAD. The
Thiago> descriptions in the ChangeLog are good enough. I'll only note
Thiago> that "keyword arguments" means calling a function specifying
Thiago> an optional argument by it's name, as in "gdb.some_function
Thiago> (foo, bar=baz)".

Thiago> Ok?

This is ok.

Do you think we should use keyword arguments universally?  I was
thinking we probably should, but I think I noticed a single-argument
function without them, and I was wondering if we should bother with
those.

Thiago> 	(valpy_binop): Use `break' to exit from the TRY_CATCH block.

I've been wishing somebody would write a gcc plugin to check cleanups
and this sort of thing as well :-)

Tom

Re: [rfa] misc fixes and improvements to Python code.

[Thiago Jung Bauermann]

El vie, 20-03-2009 a las 19:02 -0600, Tom Tromey escribió:
> >>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:
> Thiago> Ok?
> 
> This is ok.

Thanks, committed.

> Do you think we should use keyword arguments universally?  I was
> thinking we probably should, but I think I noticed a single-argument
> function without them, and I was wondering if we should bother with
> those.

Well, I added keyword arguments only to functions which have two or more
optional arguments, so that you can use the last optional argument
without having to fill the earlier ones.

We could add keyword arguments to other functions as well, but I don't
know of a practical advantage there, it would be only for simmetry, I
suppose.

> Thiago> 	(valpy_binop): Use `break' to exit from the TRY_CATCH block.
> 
> I've been wishing somebody would write a gcc plugin to check cleanups
> and this sort of thing as well :-)

Can someone write a gcc plugin adding exceptions as a syntax add-on to
the C language? :-)
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [rfa] misc fixes and improvements to Python code.

[Thiago Jung Bauermann]

El dom, 15-03-2009 a las 16:45 -0300, Thiago Jung Bauermann escribió:
> gdb/doc/
> 2009-03-15  Thiago Jung Bauermann  <bauerman@br.ibm.com>
> 	* gdb.texinfo (Values From Inferior): Fix optional arguments
> 	markup.
> 	(Commands In Python): Adjust argument names of gdb.Command.__init__
> 	to what the function accepts as keywords.

I realized I committed this patch without having approval on the
documentation part. Sorry about that. Eli, do you want me to revert the
commit?
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [rfa] misc fixes and improvements to Python code.

[Tom Tromey]

>>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:

Tom> Do you think we should use keyword arguments universally?  I was
Tom> thinking we probably should, but I think I noticed a single-argument
Tom> function without them, and I was wondering if we should bother with
Tom> those.

[...]
Thiago> We could add keyword arguments to other functions as well, but I don't
Thiago> know of a practical advantage there, it would be only for simmetry, I
Thiago> suppose.

Yeah.

I have thought about it a bit more.  I think there are two possible
benefits to applying this to all functions.  One is that it makes the
documentation simpler -- we can simply document that the argument
names in the docs are always part of the API.  The other is that it
may make some code future-proof.

Neither of these seem all that strong to me.  I think we can leave it
as-is for now.  This is the more conservative approach, anyway, in
that we can always add keyword arguments later.

Thiago> Can someone write a gcc plugin adding exceptions as a syntax add-on to
Thiago> the C language? :-)

:)

Tom

Re: [rfa] misc fixes and improvements to Python code.

[Tom Tromey]

>>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:

>> gdb/doc/
>> 2009-03-15  Thiago Jung Bauermann  <bauerman@br.ibm.com>
>> * gdb.texinfo (Values From Inferior): Fix optional arguments
>> markup.
>> (Commands In Python): Adjust argument names of gdb.Command.__init__
>> to what the function accepts as keywords.

Thiago> I realized I committed this patch without having approval on the
Thiago> documentation part. Sorry about that. Eli, do you want me to revert the
Thiago> commit?

In this specific instance, I think the documentation change is obvious.

Tom

Re: [rfa] misc fixes and improvements to Python code.

[Eli Zaretskii]

> X-Spam-Status: No, score=1.4 required=5.0 tests=DATE_IN_PAST_12_24,
> 	DNS_FROM_RFC_ABUSE autolearn=no version=3.1.0
> From: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Sun, 22 Mar 2009 23:38:06 -0300
> 
> El dom, 15-03-2009 a las 16:45 -0300, Thiago Jung Bauermann escribió:
> > gdb/doc/
> > 2009-03-15  Thiago Jung Bauermann  <bauerman@br.ibm.com>
> > 	* gdb.texinfo (Values From Inferior): Fix optional arguments
> > 	markup.
> > 	(Commands In Python): Adjust argument names of gdb.Command.__init__
> > 	to what the function accepts as keywords.
> 
> I realized I committed this patch without having approval on the
> documentation part. Sorry about that. Eli, do you want me to revert the
> commit?

No need.  It's a simple mechanical change that doesn't even need an
approval.

Thanks, and sorry I missed it the first time.