Re: [PATCH v3] Add support for creating new types from the Python API

Eli Zaretskii
2024-01-13T07:22:17Z

> From: Matheus Branco Borella <dark.ryu.550@gmail.com>
> Cc: aburgess@redhat.com,
> Matheus Branco Borella <dark.ryu.550@gmail.com>
> Date: Fri, 12 Jan 2024 22:37:57 -0300
>
> gdb/Makefile.in | 2 +
> gdb/NEWS | 16 +
> gdb/doc/python.texi | 237 ++++++++++
> gdb/python/py-float-format.c | 307 +++++++++++++
> gdb/python/py-objfile.c | 17 +
> gdb/python/py-type-init.c | 520 ++++++++++++++++++++++
> gdb/python/python-internal.h | 34 ++
> gdb/python/python.c | 50 +++
> gdb/testsuite/gdb.python/py-type-init.c | 21 +
> gdb/testsuite/gdb.python/py-type-init.exp | 132 ++++++
> 10 files changed, 1336 insertions(+)
> create mode 100644 gdb/python/py-float-format.c
> create mode 100644 gdb/python/py-type-init.c
> create mode 100644 gdb/testsuite/gdb.python/py-type-init.c
> create mode 100644 gdb/testsuite/gdb.python/py-type-init.exp

Thanks.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 11cd6c0663e..e21cca24422 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -87,6 +87,22 @@ show remote thread-options-packet
> ** New function gdb.interrupt(), that interrupts GDB as if the user
> typed control-c.
>
> + ** Functions that allow creation of instances of gdb.Type, and a new
> + class gdb.FloatFormat that may be used to create floating point
> + types. The functions that allow new type creation are:
^^
Our convention is to leave two spaces between sentences.

> +@node Creating Types In Python
> +@subsubsection Creating Types In Python
> +@cindex creating types in Python
> +@cindex Python, working with types
> +
> +@value{GDBN} makes available functionality to create new types from
> +inside Python.

This is awkward English. Suggest to reword:

@value{GDBN} allows creation of new types from Python extensions.

> +The following type creation functions are available in the @code{gdb}
> +module:

The following functions available in the @code{gdb} module create
new types:

> +@findex gdb.init_type
> +@defun gdb.init_type (owner, type_code, bit_size, name)
> +This function creates a new @code{gdb.Type} instance corresponding to a
> +type owned by the given @code{owner}, with the given type code,
> +@code{name} and size.

The arguments should have the @var markup, not @code. So:

This function creates a new @code{gdb.Type} instance corresponding to a
type owned by the given @var{owner}, with the given @var{type_code},
@var{name} and @var{bit_size}.

Likewise in the rest of this section: formal parameters should be
referenced with @var{..}, not @code{..}.

> +@code{owner} must be a reference to either a @code{gdb.Objfile} or a
> +@code{gdb.Architecture} object. These correspond to objfile and
^^
Two spaces between sentences, here and elsewhere.

> +@code{type_code} is one of the @code{TYPE_CODE_} constants defined in
> +@xref{Types In Python}.
^^^^^
This should be @ref, not @xref. @xref is only used at the beginning
of a sentence.

> +@findex gdb.init_integer_type
> +@defun gdb.init_integer_type (owner, bit_size, unsigned, name)
> +This function creates a new @code{gdb.Type} instance corresponding to an
> +integer type owned by the given @code{owner}, with the given
> +@code{name}, size and signedness.
> +
> +@code{owner} must be a reference to either a @code{gdb.Objfile} or a
> +@code{gdb.Architecture} object. These correspond to objfile and
> +architecture-owned types, respectively.
> +
> +@code{bit_size} is the size of instances of the newly created type, in
> +bits. Currently, accepted values are limited to multiples of 8.
> +
> +@code{unsigned} is a boolean indicating whether the type corresponds to
> +a signed or unsigned value.
> +
> +This function returns an instance of @code{gdb.Type}, and will throw an
> +exception in case of an error.
> +@end defun

There's no need to repeat the same text over and over again, for each
function. It should be enough to describe these arguments once, and
say that all of the functions accept these arguments. The only thing
you should say about each function is what data type it creates, all
the rest is similar or identical for all of these functions, and
should be only described once. If some functions accept arguments
that are unique to them, then describe those unique arguments as part
of the function's description. Otherwise, just refer to the general
description, which should be at the beginning of the section.

> +@findex gdb.init_pointer_type
> +@defun gdb.init_pointer_type (owner, target, bit_size, name)
> +This function creates a new @code{gdb.Type} instance corresponding to a
> +pointer type that points to @code{target} and is owned by the given
> +@code{owner}, with the given @code{name} and size.
> +
> +@code{owner} must be a reference to either a @code{gdb.Objfile} or a
> +@code{gdb.Architecture} object. These correspond to objfile and
> +architecture-owned types, respectively.
> +
> +@code{target} is a @code{gdb.Type} object, corresponding to the type
> +that will be pointed to by the newly created pointer type.
> +
> +@code{bit_size} is the size of instances of the newly created type, in
> +bits. Currently, accepted values are limited to multiples of 8.

Does this mean one can create pointer types of arbitrary length,
regardless of the target architecture? Aren't pointers limited to
what the underlying system supports?

> +@findex gdb.init_boolean_type
> +@defun gdb.init_fixed_point_type (owner, bit_size, unsigned, name)

Typo in the first line?

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