Re: [PATCH 01/10] gdb/doc: spring clean the Python unwinders documentation

[Eli Zaretskii <eliz@gnu.org>]

> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Fri, 10 Mar 2023 14:55:18 +0000
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> 
> The documentation for the Python Unwinders API could do with some
> improvement.  The 'Unwinder Skeleton Code' has an error: it says
> 'unwinders' when it should say 'unwinder' in one case.
> 
> Additionally, by placing the 'Unwinder Skeleton Code' before the
> section 'Registering an Unwinder' we have skipping including the
> registration line in the skeleton code.  But this is confusion for
> users (I think) as the skeleton code is almost complete, except for
> one missing line which the user has to figure out for themselves.  By
> reordering the sections, it is now obvious that the registration
> should be included in the skeleton code, and the example is therefore
> almost complete.
> 
> Additionally, in the example skeleton code the way in which the
> frame-id was being built (using the current stack point and program
> counter is (a) not correct, and (b) counter to what is laid out in the
> 'Unwinder Input' section when describing building a frame-id.
> 
> I've removed the incorrect code and replaced it with more generic
> comments indicating what needs to be done.  As the actual actions that
> need to be performed are both architecture specific, and dependent on
> the function being unwound, it's almost impossible to include more
> exact code here, but I think what I'm proposing is less misleading
> than what we had before.
> 
> I've also added more cross references.
> ---
>  gdb/doc/python.texi | 82 ++++++++++++++++++++++++---------------------
>  1 file changed, 44 insertions(+), 38 deletions(-)

Thanks.

>  @defun PendingFrame.create_unwind_info (frame_id)
>  Returns a new @code{gdb.UnwindInfo} instance identified by given
> -@var{frame_id}.  The argument is used to build @value{GDBN}'s frame ID
> -using one of functions provided by @value{GDBN}.  @var{frame_id}'s attributes
> -determine which function will be used, as follows:
> +@var{frame_id}.  The @var{frame_id} is used internally by @value{GDBN}
> +to identify the frames within the current thread's stack.  The
> +attributes of @var{frame_id} determine what type of frame ID is
> +created within @value{GDBN}:                        ^^^^^^^^

I think the "ID" part there should be dropped (you are talking about
creating frames, not IDs, right?).

> +Object files and program spaces can have unwinders registered with
> +them.  In addition, you can also register unwinders globally.
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Either "in addition" or "also", but not both.

> +same name.  An attempt to add an unwinder with an already existing
> +name raises an exception unless @var{replace} is @code{True}, in which
> +case the old unwinder is deleted and the new unwinder is registered in
> +its place.                                                          ^^
   ^^^^^^^^^
I believe "in its stead" is better English-wise.

> +@subheading Unwinder Precedence
> +
> +@value{GDBN} first calls the unwinders from all the object files in no
> +particular order, then the unwinders from the current program space,
> +then the globally registered unwinders, and finally the unwinders
> +builtin to @value{GDBN}.

This text is too short to justify a separate @subheading, IMO.

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