From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11573 invoked by alias); 15 Feb 2015 16:50:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 11560 invoked by uid 89); 15 Feb 2015 16:50:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: mtaout26.012.net.il Received: from mtaout26.012.net.il (HELO mtaout26.012.net.il) (80.179.55.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 15 Feb 2015 16:50:07 +0000 Received: from conversion-daemon.mtaout26.012.net.il by mtaout26.012.net.il (HyperSendmail v2007.08) id <0NJT00700NY5KE00@mtaout26.012.net.il> for gdb-patches@sourceware.org; Sun, 15 Feb 2015 18:50:18 +0200 (IST) Received: from HOME-C4E4A596F7 ([87.69.4.28]) by mtaout26.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0NJT005VPO3UCE20@mtaout26.012.net.il>; Sun, 15 Feb 2015 18:50:18 +0200 (IST) Date: Sun, 15 Feb 2015 16:50:00 -0000 From: Eli Zaretskii Subject: Re: [PATCH] Add Guile frame filter interface In-reply-to: <87oaov5s98.fsf@igalia.com> To: Andy Wingo Cc: gdb-patches@sourceware.org, xdje42@gmail.com Reply-to: Eli Zaretskii Message-id: <83vbj3unk0.fsf@gnu.org> References: <87oaov5s98.fsf@igalia.com> X-IsSubscribed: yes X-SW-Source: 2015-02/txt/msg00358.txt.bz2 > From: Andy Wingo > Cc: Andy Wingo , Doug Evans > Date: Sun, 15 Feb 2015 12:27:47 +0100 > > The attached patch exposes the frame filter interface to Guile. It's > mostly modelled on the Python interface, but has a more functional > flavor. No test cases yet, but I figure there will be a round of > comments so it's probably worth getting this out early. > > Thoughts? A few comments regarding the documentation part: First, I think this warrants a NEWS entry. > +@node Guile Frame Filter API > +@subsubsection Filtering Frames in Guile > +@cindex frame filters api I would add ", guile" to the index entry, to make it more specific. > +are affected. The commands that work with frame filters are: > + > +@code{backtrace} (@pxref{backtrace-command,, The backtrace command}), > +@code{-stack-list-frames} > +(@pxref{-stack-list-frames,, The -stack-list-frames command}), > +@code{-stack-list-variables} (@pxref{-stack-list-variables,, The > +-stack-list-variables command}), @code{-stack-list-arguments} > +@pxref{-stack-list-arguments,, The -stack-list-arguments command}) and > +@code{-stack-list-locals} (@pxref{-stack-list-locals,, The > +-stack-list-locals command}). I don't really like this style of "itemized list with cross-references", IMO it looks ugly. Please consider using some more traditional format, like a @table maybe. But if you like the results of this, I won't argue more. > +reorganize, insert, and remove frames. @value{GDBN} also provides a > +more simple @dfn{frame annotator} API that works on individual frames, In general, any thing you put inside @dfn should have a @cindex entry. > +There can be multiple frame filters registered with @value{GDBN}, and > +each one may be individually enabled or disabled at will. Multiple > +frame filters can be enabled at the same time. Frame filters have an > +associated @dfn{priority} which determines the order in which they are Here, I question the need for having "priority" in @dfn: it's not new terminology. > +should be a number, and which defaults to 20 if not given. By > +default, the filter is @dfn{global}, meaning that it is associated Likewise here for "global". > +annotated frame is always associated with a GDB frame object. To ^^^ @value{GDBN} > +object that inherits all fields from @var{x}, but whose function name > +has been set to @code{"foo"}. I find the results of @code{"foo"} to be awkward, at least in the Info manual. I suggest to use @samp{foo} instead. (I understand that you wanted to make it clear this is a string, but I think these double quotes are not really necessary, as everybody understands that it's a string from what the text says.) > +@deffn {Scheme Procedure} annotated-frame-arguments ann > +Return a list of the function arguments associated with the annotated > +frame @var{ann}. Each item of the list should either be a GDB symbol > +(@pxref{Symbols In Guile}), a pair of a GDB symbol and a GDB value > +(@pxref{Values From Inferior In Guile}, or a pair of a string and a > +GDB value. In the first case, the value will be loaded from the frame > +if needed. @value{GDBN}, 4 times > +Annotated frames may also have @dfn{child frames}. By default, no > +frame has a child frame, but filters may reorganize the frame stream > +into a stream of frame trees, by populating the child list. Of > +course, such a reorganization is ultimately cosmetic, as it doesn't > +alter the stack of frames seen by @value{GDBN} and navigable by the > +user, for example by using the @code{frame} command. Still, nesting > +frames may lead to a more understandable presentation of a backtrace. > + > +@deffn {Scheme Procedure} annotated-frame-children ann > +Return a list of the @dfn{child frames} function name associated with Even the first instance of @dfn{child frames} is questionable, IMO; having 2 of them is way too much. > +While frame filters can both reorganize and reannotate the frame > +stream, it is often the case that one only wants to reannotate the > +frames in a stream, without reorganizing then. In that case there is > +a simpler API for @dfn{frame annotators} that simply maps annotated > +frames to annotated frames. You already introduced "frame annotators" earlier, so no need to use @dfn here. > +@value{GDBN}. @var{annotator} should be a function of one argument, > +takingn annotated frame object and returning a possibily modified ^^^^^^^ A typo. > +@node Writing a Frame Filter in Guile > +@subsubsection Writing a Frame Filter in Guile > +@cindex writing a frame filter This @cindex entry should be qualified with "in guile" or some such. > +the case, because unlike normal Scheme procedures, @code{stream-cons} > +is @dfn{lazy} in its arguments, which is to say that its arguments are Again, @dfn is wrong here. In all the cases where I commented on incorrect @dfn usage, if what you wanted was to have the text in italics, just use @emph instead. Thanks.