Re: [PATCH v1] [gdb/infcmd]: Add next-expression command

[Eli Zaretskii <eliz@gnu.org>]

> Cc: Simon Farre <simon.farre.cx@gmail.com>
> Date: Tue, 16 May 2023 11:47:11 +0200
> From: Simon Farre via Gdb-patches <gdb-patches@sourceware.org>
> 
>  gdb/NEWS               |   6 ++
>  gdb/buildsym-legacy.c  |   4 +-
>  gdb/buildsym-legacy.h  |   2 +-
>  gdb/buildsym.c         |   3 +-
>  gdb/buildsym.h         |   4 +-
>  gdb/coffread.c         |   4 +-
>  gdb/dbxread.c          |   6 +-
>  gdb/doc/gdb.texinfo    |  16 +++++-
>  gdb/doc/python.texi    |   5 ++
>  gdb/dwarf2/read.c      |  31 ++++++----
>  gdb/infcmd.c           | 128 ++++++++++++++++++++++++++++++++++++++++-
>  gdb/mdebugread.c       |   2 +-
>  gdb/python/py-symtab.c |  25 +++++---
>  gdb/stack.c            |   7 +++
>  gdb/symtab.c           |   1 +
>  gdb/symtab.h           |  27 +++++++++
>  16 files changed, 238 insertions(+), 33 deletions(-)

Thanks.

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,12 @@
>  
>  *** Changes since GDB 13
>  
> +* Added the "next-expression" ("ne") command that source-level steps to
> +  columns if the required debug info metadata was emitted by the compiler.
> +

This is not the usual style of NEWS when describing new commands.  We
usually say 'New command "next-expression" which ...".

Also, I don't think I understand this description.  What did you mean
by "source-level steps to columns"?

> +* Added column attribute to Python type Symtab_and_line to reflect the added
> +  column field on "linetable_entry" which is used in "next-expression" command.
> +

The usual style is something like "New 'column' attribute of Python
type Symtab_and_line...".

> +@kindex next-expression
> +@kindex ne @r{(@code{next-expression})}
> +@item next-expression @r{[}@var{count}@r{]}
> +Continue to the next expression or statement on the current source line

This begs the question what happens if we are already on the last
expression or statement.

> +in the current (innermost) stack frame. This is a source-level command
> +and as such requires that debug information was emitted by the compiler. If
> +no such debug information could be found this command defaults to
> +@code{next}.

It would be good to mention the version of DWARF2 that supports the
necessary information, and perhaps also the compilers known to emit
it.  At least for GCC, I'd certainly like to see this in the manual.

> +An argument @var{count} is a repeat count, as for @code{next}. If
   ^^^^^^^^^^^^^^^^^^^^^^^                                      ^^
"The argument @var{count}".  Also, please leave two spaces between
sentences, per our conventions.

> +repeat count is larger than amount of expressions on the current source line
> +the command will default to the @code{next} command.

I suggest to reword:

  If the current source line doesn't have enough expressions to
  satisfy @var{count}, this command will do the same as @code{next}.

> @@ -31313,7 +31327,7 @@ An -exec-until or similar CLI command was accomplished.
>  @item watchpoint-scope
>  A watchpoint has gone out of scope.
>  @item end-stepping-range
> -An -exec-next, -exec-next-instruction, -exec-step, -exec-step-instruction or 
> +An -exec-next, -exec-next-instruction, -exec-step, -exec-step-instruction or
>  similar CLI command was accomplished.
>  @item exited-signalled 
>  The inferior exited because of a signal.

This hunk is an unrelated whitespace change.

> +@defvar Symtab_and_line.column
> +Indicates the current column number for this object. This
> +attribute is not writeable.                        ^^

Two spaces there.  More importantly, what does current column measure
in this case?  Is that a character number from beginning of source
line, a byte number since the beginning of line, or the visual column
(which can be different from the character number if some characters
are double-width)?  How many columns is a TAB?

IOW, we must explain the manual what we mean by "column" here.  It
isn't trivial.

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