[4/4] gdb/python: allow Frame.read_var to accept named arguments

Message ID e1c96ad837585b98da380739a3a814b9ea0c9785.1680177890.git.aburgess@redhat.com
State New
Headers
Series Python API: Accept named arguments in a few more places |

Commit Message

Andrew Burgess March 30, 2023, 12:10 p.m. UTC
  This commit allows Frame.read_var to accept named arguments, and also
improves (I think) some of the error messages emitted when values of
the wrong type are passed to this function.

The read_var method takes two arguments, one a variable, which is
either a gdb.Symbol or a string, while the second, optional, argument
is always a gdb.Block.

I'm now using 'O!' as the format specifier for the second argument,
which allows the argument type to be checked early on.  Currently, if
the second argument is of the wrong type then we get this error:

  (gdb) python print(gdb.selected_frame().read_var("a1", "xxx"))
  Traceback (most recent call last):
    File "<string>", line 1, in <module>
  RuntimeError: Second argument must be block.
  Error while executing Python code.
  (gdb)

After this commit, we now get an error like this:

  (gdb) python print(gdb.selected_frame().read_var("a1", "xxx"))
  Traceback (most recent call last):
    File "<string>", line 1, in <module>
  TypeError: argument 2 must be gdb.Block, not str
  Error while executing Python code.
  (gdb)

Changes are:

  1. Exception type is TypeError not RuntimeError, this is unfortunate
  as user code _could_ be relying on this, but I think the improvement
  is worth the risk, user code relying on the exact exception type is
  likely to be pretty rare,

  2. New error message gives argument position and expected argument
  type, as well as the type that was passed.

If the first argument, the variable, has the wrong type then the
previous exception was already a TypeError, however, I've updated the
text of the exception to more closely match the "standard" error
message we see above.  If the first argument has the wrong type then
before this commit we saw this:

  (gdb) python print(gdb.selected_frame().read_var(123))
  Traceback (most recent call last):
    File "<string>", line 1, in <module>
  TypeError: Argument must be a symbol or string.
  Error while executing Python code.
  (gdb)

And after we see this:

  (gdb) python print(gdb.selected_frame().read_var(123))
  Traceback (most recent call last):
    File "<string>", line 1, in <module>
  TypeError: argument 1 must be gdb.Symbol or str, not int
  Error while executing Python code.
  (gdb)

For existing code that doesn't use named arguments and doesn't rely on
exceptions, there will be no changes after this commit.
---
 gdb/doc/python.texi                   |  2 +-
 gdb/python/py-frame.c                 | 28 ++++++++++++----------
 gdb/testsuite/gdb.python/py-frame.exp | 34 +++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 14 deletions(-)
  

Comments

Eli Zaretskii March 30, 2023, 2:11 p.m. UTC | #1
> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Thu, 30 Mar 2023 13:10:23 +0100
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> 
> -@defun Frame.read_var (variable @r{[}, block@r{]})
> +@defun Frame.read_var (@var{variable} @r{[}, @var{block}@r{]})

And here.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  

Patch

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index f54909cc05d..881c743034e 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -5406,7 +5406,7 @@ 
 object.
 @end defun
 
-@defun Frame.read_var (variable @r{[}, block@r{]})
+@defun Frame.read_var (@var{variable} @r{[}, @var{block}@r{]})
 Return the value of @var{variable} in this frame.  If the optional
 argument @var{block} is provided, search for the variable from that
 block; otherwise start at the frame's current block (which is
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 3a033ac7570..082358effab 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -475,15 +475,18 @@  frapy_find_sal (PyObject *self, PyObject *args)
    gdb.Symbol.  The block argument must be an instance of gdb.Block.  Returns
    NULL on error, with a python exception set.  */
 static PyObject *
-frapy_read_var (PyObject *self, PyObject *args)
+frapy_read_var (PyObject *self, PyObject *args, PyObject *kw)
 {
   frame_info_ptr frame;
   PyObject *sym_obj, *block_obj = NULL;
   struct symbol *var = NULL;	/* gcc-4.3.2 false warning.  */
   const struct block *block = NULL;
 
-  if (!PyArg_ParseTuple (args, "O|O", &sym_obj, &block_obj))
-    return NULL;
+  static const char *keywords[] = { "variable", "block", nullptr };
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "O|O!", keywords,
+					&sym_obj, &block_object_type,
+					&block_obj))
+    return nullptr;
 
   if (PyObject_TypeCheck (sym_obj, &symbol_object_type))
     var = symbol_object_to_symbol (sym_obj);
@@ -495,15 +498,13 @@  frapy_read_var (PyObject *self, PyObject *args)
       if (!var_name)
 	return NULL;
 
-      if (block_obj)
+      if (block_obj != nullptr)
 	{
+	  /* This call should only fail if the type of BLOCK_OBJ is wrong,
+	     and we ensure the type is correct when we parse the arguments,
+	     so we can just assert the return value is not nullptr.  */
 	  block = block_object_to_block (block_obj);
-	  if (!block)
-	    {
-	      PyErr_SetString (PyExc_RuntimeError,
-			       _("Second argument must be block."));
-	      return NULL;
-	    }
+	  gdb_assert (block != nullptr);
 	}
 
       try
@@ -533,8 +534,9 @@  frapy_read_var (PyObject *self, PyObject *args)
     }
   else
     {
-      PyErr_SetString (PyExc_TypeError,
-		       _("Argument must be a symbol or string."));
+      PyErr_Format (PyExc_TypeError,
+		    _("argument 1 must be gdb.Symbol or str, not %s"),
+		    Py_TYPE (sym_obj)->tp_name);
       return NULL;
     }
 
@@ -787,7 +789,7 @@  Return the frame called by this frame." },
   { "find_sal", frapy_find_sal, METH_NOARGS,
     "find_sal () -> gdb.Symtab_and_line.\n\
 Return the frame's symtab and line." },
-  { "read_var", frapy_read_var, METH_VARARGS,
+  { "read_var", (PyCFunction) frapy_read_var, METH_VARARGS | METH_KEYWORDS,
     "read_var (variable) -> gdb.Value.\n\
 Return the value of the variable in this frame." },
   { "select", frapy_select, METH_NOARGS,
diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp
index 07997326d09..16177c8a5f8 100644
--- a/gdb/testsuite/gdb.python/py-frame.exp
+++ b/gdb/testsuite/gdb.python/py-frame.exp
@@ -45,6 +45,10 @@  gdb_test "python print (bf1.read_var(\"i\"))" "\"stuff\"" "test i"
 gdb_test "python print (bf1.read_var(\"f\"))" "\"foo\"" "test f"
 gdb_test "python print (bf1.read_var(\"b\"))" "\"bar\"" "test b"
 
+# Check we can use a single named argument with read_var.
+gdb_test "python print (bf1.read_var(variable = \"b\"))" "\"bar\"" \
+    "test b using named arguments"
+
 # Test the read_var function in another block other than the current
 # block (in this case, the super block). Test thar read_var is reading
 # the correct variables of i and f but they are the correct value and type.
@@ -54,12 +58,42 @@  gdb_test "python print (bf1.read_var(\"i\", sb).type)" "double" "test double i"
 gdb_test "python print (bf1.read_var(\"f\", sb))" "2.2.*" "test f = 2.2"
 gdb_test "python print (bf1.read_var(\"f\", sb).type)" "double" "test double f"
 
+# Now test read_var with a variable and block using named arguments.
+gdb_test "python print (bf1.read_var(block = sb, variable = \"i\"))" "1.1.*" \
+    "test i = 1.1 usign named arguments"
+gdb_test "python print (bf1.read_var(block = sb, variable = \"f\"))" "2.2.*" \
+    "test f = 2.2 using named arguments"
+
 # And again test another outerblock, this time testing "i" is the
 # correct value and type.
 gdb_py_test_silent_cmd "python sb = sb.superblock" "get superblock" 0
 gdb_test "python print (bf1.read_var(\"i\", sb))" "99" "test i = 99"
 gdb_test "python print (bf1.read_var(\"i\", sb).type)" "int" "test int i"
 
+# Test what happens when we provide a block of the wrong type.
+gdb_test "python print (bf1.read_var(\"i\", \"some_block\"))" \
+    [multi_line \
+	 "TypeError: argument 2 must be gdb\\.Block, not str" \
+	 "Error while executing Python code\\."] \
+    "check invalid block type error"
+gdb_test "python print (bf1.read_var(block = \"some_block\", variable = \"i\"))" \
+    [multi_line \
+	 "TypeError: argument 2 must be gdb\\.Block, not str" \
+	 "Error while executing Python code\\."] \
+    "check invalid block type error when named args are used"
+
+# Test what happens when we provide a variable of the wrong type.
+gdb_test "python print (bf1.read_var(None))" \
+    [multi_line \
+	 "TypeError: argument 1 must be gdb\\.Symbol or str, not NoneType" \
+	 "Error while executing Python code\\."] \
+    "check read_var error when variable is None"
+gdb_test "python print (bf1.read_var(sb))" \
+    [multi_line \
+	 "TypeError: argument 1 must be gdb\\.Symbol or str, not gdb\\.Block" \
+	 "Error while executing Python code\\."] \
+    "check read_var error when variable is a gdb.Block"
+
 gdb_breakpoint "f2"
 gdb_continue_to_breakpoint "breakpoint at f2"
 gdb_py_test_silent_cmd "python bframe = gdb.selected_frame()" \