public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andy Wingo <wingo@igalia.com>
To: Alexander Smundak <asmundak@google.com>
Cc: Doug Evans <dje@google.com>,  gdb-patches <gdb-patches@sourceware.org>
Subject: Re: [RFC] [PATCH] Provide the ability to write the frame unwinder in Python
Date: Tue, 17 Mar 2015 08:57:00 -0000	[thread overview]
Message-ID: <87sid4atms.fsf@igalia.com> (raw)
In-Reply-To: <CAHQ51u7KzQLSLC=QeLA=zd+TUkbbNzzndfeVLFWpjiR-pL8ang@mail.gmail.com>	(Alexander Smundak's message of "Mon, 16 Mar 2015 10:25:13 -0700")

On Mon 16 Mar 2015 18:25, Alexander Smundak <asmundak@google.com> writes:

> I'd like to propose one improvement on the Python side: UnwinderInfo
> is constructed by a frame method instead of an implicit constructor.
> I.e., frame.create_frame_with_id(sp, pc) returns UnwindInfo instance
> whose ID is the result of calling GDB's frame_id_build(sp, pc),
> frame.create_frame_with_id_wild(sp) returns UnwindInfo instance
> whose ID is the results of calling frame_id_build_wild(sp), etc.
>
> The example above would then look as follows:
>   def unwind(frame):
>     if we_can_handle(frame):
>       unwind_info = frame.create_frame_with_id(sp, pc)
>       unwind_info.set_previous_frame_register("r0", r0)
>       unwind_info.set_previous_frame_register(...)
>       return unwind_info
>     else
>       return None

Looks great to me :)  Thank you for the consideration!

I might consider naming "set_previous_frame_register" as
"add_saved_register", in anticipation of a possible
add_unavailable_register(), add_unmodified_register(), etc, but that is
just a minor nit.

> As to the class of an object passed to a sniffer, how about calling it
> FrameData? Note that it's not very important from the user's point of
> view as sniffer code does not ever reference it by name.

It's true that from user code it barely matters to Python, but Scheme's
monomorphic flavor makes these things more apparent:

  (frame-data-read-register frame "r0")

This doesn't read so well to me -- is it "read-register" on a
"frame-data", or is it "data-read-register" on a "frame" ?  A weak point
but "ephemeral-frame-read-register" avoids the question.

I also think that "ephemeral frame" is easier to document as a concept
than the more generic "frame data", and corresponds better to what is
happening in GDB.  YMMV, though.

As an aside, it seems to me that if we can avoid it, the word "sniffer"
should not enter the documentation or the API.  The Python and Guile
APIs don't just sniff, they do the whole of the unwinding operation, so
it's more clear to call them "unwinders".

Andy

  reply	other threads:[~2015-03-17  8:57 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-15 18:14 Alexander Smundak
2014-12-22 19:24 ` Alexander Smundak
2014-12-29 18:02   ` Alexander Smundak
2015-01-05 17:53     ` Alexander Smundak
2015-01-12 20:03       ` Alexander Smundak
2015-01-22  3:31         ` Alexander Smundak
2015-01-29  1:36           ` Alexander Smundak
2015-01-12 21:00 ` Simon Marchi
2015-01-12 21:22   ` Doug Evans
2015-02-04 22:36 ` Doug Evans
2015-02-12 17:58   ` Alexander Smundak
2015-02-19  2:32     ` Alexander Smundak
2015-02-20 11:12     ` Phil Muldoon
2015-02-26  3:09       ` Alexander Smundak
2015-03-02 22:56         ` Alexander Smundak
2015-03-03  8:46           ` Andy Wingo
2015-03-04  2:36             ` Alexander Smundak
2015-03-04  7:49               ` Andy Wingo
2015-03-09 11:02                 ` Phil Muldoon
2015-03-11  2:22                   ` Alexander Smundak
2015-03-11  8:49                     ` Andy Wingo
2015-03-11 17:34                       ` Doug Evans
2015-03-11 18:48                       ` Alexander Smundak
2015-03-16 11:29                         ` Andy Wingo
2015-03-16 12:01                           ` Andy Wingo
2015-03-16 17:25                           ` Alexander Smundak
2015-03-17  8:57                             ` Andy Wingo [this message]
2015-03-17 19:48                               ` Alexander Smundak
2015-03-17 21:37                                 ` Alexander Smundak
2015-03-18  8:54                                   ` Andy Wingo
2015-03-18 22:57                                     ` Alexander Smundak
2015-03-23 19:58                                       ` Doug Evans
2015-03-24  9:06                                         ` Andy Wingo
2015-03-26  3:31                                         ` Alexander Smundak
2015-03-26 18:53                                           ` Eli Zaretskii
2015-03-27 22:29                                           ` Doug Evans
2015-03-28  1:10                                             ` Alexander Smundak
2015-03-30 17:45                                               ` Doug Evans
2015-03-30 19:49                                                 ` Alexander Smundak
2015-03-31 22:36                                                   ` Doug Evans
2015-04-01  0:09                                                     ` Alexander Smundak
2015-04-01  0:28                                                       ` Doug Evans
2015-03-18 23:25                                 ` Doug Evans
2015-03-19  0:36                                   ` Alexander Smundak
2015-03-19  8:12                                     ` Andy Wingo
2015-03-20  0:15                                       ` Doug Evans
2015-03-20  2:27                                         ` Alexander Smundak
2015-03-20 17:48                                           ` Doug Evans
2015-03-20  8:26                                         ` Andy Wingo
2015-03-20 18:32                                           ` Doug Evans
2015-03-17 22:21                               ` Doug Evans
2015-03-18  8:57                                 ` Andy Wingo
2015-03-18 16:48                                   ` Doug Evans
2015-03-19  8:04                                     ` Andy Wingo
2015-03-09  9:42           ` Andy Wingo
2015-03-03  0:49         ` Alexander Smundak
2015-03-03 14:38           ` Andy Wingo
2015-03-04  2:52             ` Alexander Smundak
2015-02-20  9:42 ` Phil Muldoon
2015-02-20  9:59   ` 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=87sid4atms.fsf@igalia.com \
    --to=wingo@igalia.com \
    --cc=asmundak@google.com \
    --cc=dje@google.com \
    --cc=gdb-patches@sourceware.org \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).