From: Tom Tromey <tromey@redhat.com>
To: Phil Muldoon <pmuldoon@redhat.com>
Cc: "gdb-patches\@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [patch][python] 2 of 5 - Frame filter MI code changes.
Date: Mon, 10 Dec 2012 21:03:00 -0000 [thread overview]
Message-ID: <87sj7dd3i7.fsf@fleche.redhat.com> (raw)
In-Reply-To: <50C1F56E.5060506@redhat.com> (Phil Muldoon's message of "Fri, 07 Dec 2012 13:55:58 +0000")
>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
Tom> I'd prefer it if the various callers were changed to use mi_getopt.
Tom> This provides uniformity and lets us add options later.
Phil> If there was uniformity then I would agree, but as far as I looked
Phil> there wasn't. Some MI commands use mi_getopt, some parse their own
Phil> options, some allow long options ("--"), others do not, and mi_getopt
Phil> does not handle long options in any case (and huge amounts of other
Phil> useful getopt functions too). I wrote a patch for mi_getopts to
Phil> handle long options, but why do we even need another implementation of
Phil> getopt like functionality?
Ok, I see the problem now.
Right now, though. -stack-list-frames doesn't take any options.
I think it would be better if commands like this were converted to use
mi_getopt instead of doing the parsing by hand.
So I suggest having all the functions use a short option name.
I think the changes like in -stack-list-locals are strange, too, in that
they make options order-dependent. This will be surprising to users.
Phil> I wanted to mention something else about MI. I recently discovered in
Phil> the GDB manual that -stack-list-locals, -stack-list-arguments are
Phil> considered depreciated. Not even sure if we should add frame filter
Phil> logic to them. What do you think?
I think we still ought to. It doesn't seem like much work and it is
friendlier in case some MI user hasn't updated.
Tom> I don't think I follow the high/low logic here.
Tom> How does this code strip off the first 'frame_low' frames?
Phil> fi is unwound to the position of frame_low in a loop preceding this
Phil> call. This is existing code, and not in the patch context.
Tom> Also, Do frame_low and frame_high refer to "raw" or "cooked" frames?
Tom> I tend to think they should refer to cooked ones, but I think at least
Tom> the answer should be explicit and documented.
Phil> In the existing mi sense, they just refer to frames on the stack. I
Phil> followed this logic, but something I am still unsure of is if a frame
Phil> is elided between frame low, and frame high, if that should be
Phil> counted. I think it should.
I see. I think this is wrong then. I think the arguments have to be
applied after filtering. Here's why:
One use for these arguments to the stack commands is so that a GUI can
display just part of the stack, and then, if the user scrolls the
display, it can request more of the stack to show. This way the GUI
doesn't have to unwind the whole stack just to show the first 5 lines or
whatever.
Suppose you have a frame filter that notices a sentinel frame and then
elides the following 3 frames. This isn't necessarily an obscure
situation, maybe some interpreter takes a few function calls to
implement a call in the interpreted language and the filter wants to
collapse those frames to just show what is happening in the interpreted
code.
Suppose this sentinel frame occurs at stack position 3.
Now suppose the MI client requests frames 0..4. Ok, the filter will
work (since it can request more frames than the MI client did) and will
present the right thing.
But now the user scrolls the window and the MI client requests frames
starting at frame 5.
Whoops -- the output will be inconsistent; the filter won't trigger
because the sentinel appears at frame 3, which the existing code already
iterated past.
Tom
next prev parent reply other threads:[~2012-12-10 21:03 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-30 14:31 Phil Muldoon
2012-12-05 17:11 ` Tom Tromey
2012-12-07 13:56 ` Phil Muldoon
2012-12-10 21:03 ` Tom Tromey [this message]
2013-02-05 12:08 ` Phil Muldoon
2013-02-07 21:32 ` Tom Tromey
2013-02-20 15:17 ` Phil Muldoon
2013-02-20 20:37 ` Tom Tromey
2013-03-11 22:13 Phil Muldoon
2013-03-12 20:43 ` Tom Tromey
2013-03-12 20:52 ` Phil Muldoon
2013-03-13 12:15 ` Phil Muldoon
2013-03-13 17:48 ` Tom Tromey
2013-03-13 19:41 ` Phil Muldoon
2013-03-13 20:27 ` Tom Tromey
2013-03-13 20:53 ` Phil Muldoon
2013-03-13 20:56 ` Tom Tromey
2013-03-13 21:10 ` Phil Muldoon
2013-03-14 17:54 ` Tom Tromey
2013-03-14 19:35 ` Phil Muldoon
2013-04-22 16:01 Phil Muldoon
2013-04-26 11:19 ` Tom Tromey
2013-05-06 8:23 Phil Muldoon
2013-05-06 20:42 ` Tom Tromey
2013-05-07 8:23 ` Phil Muldoon
2013-05-07 14:02 ` Tom Tromey
2013-05-08 10:18 ` Phil Muldoon
2013-05-08 19:47 ` Tom Tromey
2013-05-10 10:45 ` Phil Muldoon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87sj7dd3i7.fsf@fleche.redhat.com \
--to=tromey@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=pmuldoon@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox