[PATCH] Use the "O!" format more in the Python code

[PATCH] Use the "O!" format more in the Python code

[Tom Tromey]

I noticed a few spots in the Python code that were decoding method
arguments and then immediately type-checking an argument.  This can be
done more concisely using the "O!" format.
---
 gdb/python/py-disasm.c        | 11 ++---------
 gdb/python/py-prettyprint.c   | 11 ++++-------
 gdb/python/py-record-btrace.c |  4 +---
 gdb/python/py-symbol.c        |  8 +-------
 4 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index 748ffd5e6a3..7635e45db56 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -620,17 +620,10 @@ disasmpy_set_enabled (PyObject *self, PyObject *args, PyObject *kw)
 {
   PyObject *newstate;
   static const char *keywords[] = { "state", nullptr };
-  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "O", keywords,
-					&newstate))
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "O!", keywords,
+					&PyBool_Type, &newstate))
     return nullptr;
 
-  if (!PyBool_Check (newstate))
-    {
-      PyErr_SetString (PyExc_TypeError,
-		       _("The value passed to `_set_enabled' must be a boolean."));
-      return nullptr;
-    }
-
   python_print_insn_enabled = newstate == Py_True;
   Py_RETURN_NONE;
 }
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index 0aa11d29094..eb7ac2c048f 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -679,15 +679,12 @@ gdbpy_default_visualizer (PyObject *self, PyObject *args)
   PyObject *val_obj;
   struct value *value;
 
-  if (! PyArg_ParseTuple (args, "O", &val_obj))
+  if (! PyArg_ParseTuple (args, "O!", &value_object_type, &val_obj))
     return NULL;
   value = value_object_to_value (val_obj);
-  if (! value)
-    {
-      PyErr_SetString (PyExc_TypeError,
-		       _("Argument must be a gdb.Value."));
-      return NULL;
-    }
+  /* This was ensured by the type-checking during argument
+     parsing.  */
+  gdb_assert (value != nullptr);
 
   return find_pretty_printer (val_obj).release ();
 }
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index 1974dd3e939..6026de91e67 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -922,11 +922,9 @@ recpy_bt_goto (PyObject *self, PyObject *args)
   if (tinfo == NULL || btrace_is_empty (tinfo))
 	return PyErr_Format (gdbpy_gdb_error, _("Empty branch trace."));
 
-  if (!PyArg_ParseTuple (args, "O", &parse_obj))
+  if (!PyArg_ParseTuple (args, "O!", &recpy_insn_type, &parse_obj))
     return NULL;
 
-  if (Py_TYPE (parse_obj) != &recpy_insn_type)
-    return PyErr_Format (PyExc_TypeError, _("Argument must be instruction."));
   obj = (const recpy_element_object *) parse_obj;
 
   try
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index e8ff2dfe28a..15fcbeea9be 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -266,15 +266,9 @@ sympy_value (PyObject *self, PyObject *args)
   frame_info_ptr frame_info = NULL;
   PyObject *frame_obj = NULL;
 
-  if (!PyArg_ParseTuple (args, "|O", &frame_obj))
+  if (!PyArg_ParseTuple (args, "|O!", &frame_object_type, &frame_obj))
     return NULL;
 
-  if (frame_obj != NULL && !PyObject_TypeCheck (frame_obj, &frame_object_type))
-    {
-      PyErr_SetString (PyExc_TypeError, "argument is not a frame");
-      return NULL;
-    }
-
   SYMPY_REQUIRE_VALID (self, symbol);
   if (symbol->loc_class () == LOC_TYPEDEF)
     {
-- 
2.49.0

Re: [PATCH] Use the "O!" format more in the Python code

[Tom de Vries]

On 2/27/26 5:48 PM, Tom Tromey wrote:
> -  if (!PyArg_ParseTuple (args, "O", &parse_obj))
> +  if (!PyArg_ParseTuple (args, "O!", &recpy_insn_type, &parse_obj))
>       return NULL;
>   
> -  if (Py_TYPE (parse_obj) != &recpy_insn_type)
> -    return PyErr_Format (PyExc_TypeError, _("Argument must be instruction."));

Hi,

I reviewed this patch, and LGTM.

Reviewed-By: Tom de Vries <tdevries@suse.de>

I asked an AI to review this as well, and it mentioned that the piece of 
code above slightly changes semantics because using 'O!' also allows 
subclasses.

I saw other direct comparisons to recpy_insn_type in the code, and 
wondered if those should likewise allow subclasses using PyObject_TypeCheck.

But I suppose we just assume there are no subclasses?

Thanks,
- Tom

Re: [PATCH] Use the "O!" format more in the Python code

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> I saw other direct comparisons to recpy_insn_type in the code, and
Tom> wondered if those should likewise allow subclasses using
Tom> PyObject_TypeCheck.

Tom> But I suppose we just assume there are no subclasses?

I'm not sure if you can subclass an instruction type (I thought perhaps
we needed special code in the type definition to support subclassing),
but if you can, then it should be usable here.

Tom

Re: [PATCH] Use the "O!" format more in the Python code

[Tom Tromey]

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

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> I saw other direct comparisons to recpy_insn_type in the code, and
Tom> wondered if those should likewise allow subclasses using
Tom> PyObject_TypeCheck.

Tom> But I suppose we just assume there are no subclasses?

> I'm not sure if you can subclass an instruction type (I thought perhaps
> we needed special code in the type definition to support subclassing),
> but if you can, then it should be usable here.

I answered this backward somehow!?

Yeah, if subclasses are at all possible then the other spots should be
updated.

Tom

Re: [PATCH] Use the "O!" format more in the Python code

[Tom de Vries]

On 2/27/26 8:01 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> Tom> I saw other direct comparisons to recpy_insn_type in the code, and
> Tom> wondered if those should likewise allow subclasses using
> Tom> PyObject_TypeCheck.
> 
> Tom> But I suppose we just assume there are no subclasses?
> 
>> I'm not sure if you can subclass an instruction type (I thought perhaps
>> we needed special code in the type definition to support subclassing),
>> but if you can, then it should be usable here.
> 
> I answered this backward somehow!?
> 
> Yeah, if subclasses are at all possible then the other spots should be
> updated.
I found this ( 
https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_subclasses 
), so I suppose unless tp_subclasses is set somehow in the type 
definition, there are no subclasses, and that's the case for 
recpy_insn_type.

So there's no direct need to update those other spots, but OTOH it can't 
hurt I suppose.

Thanks,
- Tom

Re: [PATCH] Use the "O!" format more in the Python code

[Paul Koning]

> On Feb 27, 2026, at 1:31 PM, Tom Tromey <tom@tromey.com> wrote:
> 
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> I saw other direct comparisons to recpy_insn_type in the code, and
> Tom> wondered if those should likewise allow subclasses using
> Tom> PyObject_TypeCheck.
> 
> Tom> But I suppose we just assume there are no subclasses?
> 
> I'm not sure if you can subclass an instruction type (I thought perhaps
> we needed special code in the type definition to support subclassing),
> but if you can, then it should be usable here.
> 
> Tom

Agreed.  The Python Way is that subclasses of a type can be used where a given type is expected.  Also, any type can be subclassed.  That last point is not true for types defined in C code, unless the code allows it to happen.  In general it should do so; it's very rare indeed for it to be "the right thing" for a type not to allow subclasses.

	paul

Re: [PATCH] Use the "O!" format more in the Python code

[Tom Tromey]

>>>>> "Paul" == Paul Koning <paulkoning@comcast.net> writes:

Paul> Also, any type can be subclassed.  That
Paul> last point is not true for types defined in C code, unless the code
Paul> allows it to happen.  In general it should do so; it's very rare
Paul> indeed for it to be "the right thing" for a type not to allow
Paul> subclasses.

In gdb we've been fairly selective with this.

I looked it up and you have to set Py_TPFLAGS_BASETYPE, which we do in
13 of the classes that gdb implements.

Maybe we should do this everywhere?  But if we did that then wouldn't we
also want to implement the 'dict' feature everywhere as well?

I'm not sure there's a big advantage to it or need for it though.
So maybe it's just busy-work.


I've been hoping to transition the internal types to be real C++ types,
like give them constructors and destructors, initializers, etc.  But
maybe it's not really possible with the Python lifetime model, at least
when subclassing is available.

Tom

Re: [PATCH] Use the "O!" format more in the Python code

[Paul Koning]

> On Feb 27, 2026, at 2:58 PM, Tom Tromey <tom@tromey.com> wrote:
> 
>>>>>> "Paul" == Paul Koning <paulkoning@comcast.net> writes:
> 
> Paul> Also, any type can be subclassed.  That
> Paul> last point is not true for types defined in C code, unless the code
> Paul> allows it to happen.  In general it should do so; it's very rare
> Paul> indeed for it to be "the right thing" for a type not to allow
> Paul> subclasses.
> 
> In gdb we've been fairly selective with this.
> 
> I looked it up and you have to set Py_TPFLAGS_BASETYPE, which we do in
> 13 of the classes that gdb implements.
> 
> Maybe we should do this everywhere?  But if we did that then wouldn't we
> also want to implement the 'dict' feature everywhere as well?

You mean in the sense that an object acts as if it has a dictionary inside, holding its attributes?  I'm not sure if that is needed for the GDB (base) classes.  Consider "slotted" objects, which don't have one, but can be subclassed and the subclass objects may have a dictionary, or not, at the discretion of the subclass author.

> I'm not sure there's a big advantage to it or need for it though.
> So maybe it's just busy-work.
> 
> I've been hoping to transition the internal types to be real C++ types,
> like give them constructors and destructors, initializers, etc.  But
> maybe it's not really possible with the Python lifetime model, at least
> when subclassing is available.

Python classes have construtors and destructors, but they work differently than C++.  Especially destructors because it isn't necessarily obvious when they will be called, at least for some scenarios where things aren't freed until the garbage collector gets there.

It's indeed possible that for some classes subclassing is unlikely to be wanted.  But I do remember, some years ago, wanting to subclass a GDB type and finding I could not.  I no longer remember which class it was, unfortunately.

	paul