[armedbear-devel] [REVISED] Patch for including relevant java stack frames
evenson at panix.com
Thu Jul 30 17:14:52 UTC 2009
On 7/30/09 12:41 PM, Tobias C. Rittweiler wrote:
>> Comments please on whether this functionality would be useful, and
>> potential improvements.
> a) Why did you change SYS:BACKTRACE-AS-LIST to listify the frames? I
> deliberately changed it to return a list of StackFrame objects, so
> higher levels can decide whether they want to print java frames, or
> not. (Which addresses your concern about "abstraction barrier".)
From what I recall this was to support the use of the :frame and
:inspect methods in the top-level REPL as it worked before. One could
select a frame in the backtrace via ":frame n" (where n was the frame
you were interested in), and then issue an ":inspect *" to start walking
through the frame. This should certainly be kept for the LispStackFrame
but maybe not for the JavaStackFrame. I think I ran into one problem
that the "~S" representation of these objects was pretty opaque (i.e.
just "#<JAVA-STACK-FRAME>" and "#<LISP-STACK-FRAME"), but I am not quite
sure of my reasoning right now. I'll revisit this in course of the
renamings you suggest below.
> Perhaps because the name BACKTRACE-AS-LIST could be understand to do
> implicit listification? I could follow that line. In fact, I'd have
> liked to perform the following renamings, but opted out to change too
> many things with my patch:
> Rename BACKTRACE to PRINT-BACKTRACE
> Define BACKTRACE to return a list of StackFrames.
> Define BACKTRACE-AS-LIST to return a list of listified stack
That sounds quite reasonable. Sure, it is a lot of renaming, but other
than SLIME, I think our only customer is 'top-level.lisp', so as long as
we preserve the behavior there we should be fine. I completely agree
that calling our main backtrace function BACKTRACE-AS-LIST is wrong.
> b) I don't like that
> (frame-to-list #<JAVA-STACK-FRAME>) => ("class.meth(file.java:NN)")
> I think it should return ("class.meth" :file "file.java" :line NN) so
> higher levels can use that information. (For example `v' in SLDB.)
Agreed. I had a version doing plist like things as well, which makes a
lot more sense for tools further down the line. I was going to make the
whole result a plist so as not to trip up future reorderings. Would you
just have the first element of the list be a string?
> c) PRINT-FRAME contains an IGNORE-ERRORS which purpose I don't
> understand. Mind you, you actually copied that from my patch, so it's
> all my fault---I just can't remember why I put it in there. Do you
> know? If so, please add a comment.
Nope, I thought *you* had the deep reason here, so I was just slaving
away via cut and paste. I'll remove it in the next iteration.
"A screaming comes across the sky. It has happened before, but there
is nothing to compare to it now."
More information about the armedbear-devel