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 frames.
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.