diff mbox

[PATCHv5,0/2] gdb: Change how frames are selected for 'frame' and 'info frame'.

Message ID 20180828084332.GI32506@embecosm.com
State New
Headers show

Commit Message

Andrew Burgess Aug. 28, 2018, 8:43 a.m. UTC 
* Eli Zaretskii <eliz@gnu.org> [2018-08-27 18:23:37 +0300]:

> > Date: Mon, 27 Aug 2018 12:03:54 +0100
> > From: Andrew Burgess <andrew.burgess@embecosm.com>
> > Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>,
> > 	Eli Zaretskii <eliz@gnu.org>
> > 
> > Eli:
> > 
> >   In this message:
> >       https://sourceware.org/ml/gdb-patches/2018-07/msg00670.html
> >   Philippe highlighted that you might have some reservations about
> >   this patch series, which I think is currently the main blocker for
> >   this patch getting approval.
> > 
> >   In the thread started here:
> >       https://sourceware.org/ml/gdb-patches/2018-05/msg00299.html
> >   and ending here:
> >       https://sourceware.org/ml/gdb-patches/2018-06/msg00142.html
> >   you did review and approve one of the original patch variants, which
> >   is most like the "level" variant of the patch submitted here:
> >       https://sourceware.org/ml/gdb-patches/2018-08/msg00337.html
> > 
> >   I would be really grateful if you could let me know your current
> >   thoughts on this patch, are you happy to have the "level" variant
> >   merged based on your previous approval, or has you position changed?
> 
> I just said
> 
>   I question the wisdom of changing such veteran terminology.
> 
> And then:
> 
>   If we are going to make this change, then I would suggest to keep the
>   index entry, _add_ to it an entry about "frame level", and explain
>   here what that level is, something like this:
> 
>     @value{GDBN} labels each existing stack frame with a @dfn{level}, a
>     number that is zero for the innermost frame, one for the frame that
>     called it, and so on upward.  These level numbers give you a way of
>     designating stack frames in @value{GDBN} commands.
> 
> IOW, I urged us to think whether we really want the change (and
> invited others to comment).  Then I had a small suggestion for if we
> do make the change.
> 
> That is all.  It was never my intention to block the patch, and if my
> words were unfortunate enough to be interpreted that way, I apologize.

Thanks for your feedback.

I didn't mean you were intentionally blocking the patch, rather, there
was some uncertainty, and _that_ was blocking.

I understand your concerns about changing existing terminology, and I
agree that this is a real issue that should be considered.

I like your proposed new paragraph as it mentions both the word
'level' and the word 'number', and I wondered if we couldn't sidestep
the issue of which term is "correct", at least for a little while,
with something like this:

    @value{GDBN} labels each existing stack frame with a @dfn{level}, a
    number that is zero for the innermost frame, one for the frame that
    called it, and so on upward.  These level numbers give you a way of
    designating stack frames in @value{GDBN} commands.  The terms
    @dfn{frame number} and @dfn{frame level} can be used interchangably to
    describe this number.

I'd also propose we add both 'frame number' and 'frame level' to the
concept index (see patch below).  Do you think that if I added this
chunk to the existing level patch we would have something that was
clear enough to merge?

Thanks,
Andrew

Comments

Eli Zaretskii Aug. 28, 2018, 9:08 a.m. UTC | #1 
> Date: Tue, 28 Aug 2018 09:43:32 +0100
> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: gdb-patches@sourceware.org, philippe.waroquiers@skynet.be
> 
> I like your proposed new paragraph as it mentions both the word
> 'level' and the word 'number', and I wondered if we couldn't sidestep
> the issue of which term is "correct", at least for a little while,
> with something like this:
> 
>     @value{GDBN} labels each existing stack frame with a @dfn{level}, a
>     number that is zero for the innermost frame, one for the frame that
>     called it, and so on upward.  These level numbers give you a way of
>     designating stack frames in @value{GDBN} commands.  The terms
>     @dfn{frame number} and @dfn{frame level} can be used interchangably to
>     describe this number.
> 
> I'd also propose we add both 'frame number' and 'frame level' to the
> concept index (see patch below).  Do you think that if I added this
> chunk to the existing level patch we would have something that was
> clear enough to merge?

Yes, thanks.
diff mbox

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a2e405d5dfc..694f2ca5e95 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7393,12 +7393,14 @@ 
 in a register called the @dfn{frame pointer register}
 (@pxref{Registers, $fp}) while execution is going on in that frame.
 
+@cindex frame level
 @cindex frame number
-@value{GDBN} assigns numbers to all existing stack frames, starting with
-zero for the innermost frame, one for the frame that called it,
-and so on upward.  These numbers do not really exist in your program;
-they are assigned by @value{GDBN} to give you a way of designating stack
-frames in @value{GDBN} commands.
+@value{GDBN} labels each existing stack frame with a @dfn{level}, a
+number that is zero for the innermost frame, one for the frame that
+called it, and so on upward.  These level numbers give you a way of
+designating stack frames in @value{GDBN} commands.  The terms
+@dfn{frame number} and @dfn{frame level} can be used interchangably to
+describe this number.
 
 @c The -fomit-frame-pointer below perennially causes hbox overflow
 @c underflow problems.