From: Attila L. <att...@gm...> - 2007-01-06 11:15:02
Attachments:
sbcl-debug.diff
|
ZGVhciBsaXN0LAoKcGxlYXNlIGZpbmQgYSBwYXRjaCBhdHRhY2hlZCB0aGF0IGNsZWFucyB1cCB0 aGUgcmVuZGVyaW5nIG9mIHRoZQpzYi1wY2w6ZmFzdC1tZXRob2QncyBpbiB0aGUgKHNsaW1lKSBi YWNrdHJhY2UuIGFsc28gcHJvcGFnYXRlcyB0aGUKdmVyYm9zaXR5IHBhcmFtZXRlciBhbmQgZGlz YWJsZXMgYW55IGNsZWFudXBzIHdoZW4gdmVyYm9zaXR5IGlzCmdyZWF0ZXIgdGhlbiAxLgoKLS0g Ci0gYXR0aWxhCgoiLSBUaGUgdHJ1dGggaXMgdGhhdCBJJ3ZlIGJlZW4gdG9vIGNvbnNpZGVyYXRl LCBhbmQgc28gYmVjYW1lCnVuaW50ZW50aW9uYWxseSBjcnVlbC4uLgogLSBJIHVuZGVyc3RhbmQu CiAtIE5vLCB5b3UgZG9uJ3QgdW5kZXJzdGFuZCEgV2UgZG9uJ3Qgc3BlYWsgdGhlIHNhbWUgbGFu Z3VhZ2UhIgooSW5nbWFyIEJlcmdtYW4gLSBTbXVsdHJvbnN0w6RsbGV0KQo= |
From: Attila L. <att...@gm...> - 2007-04-10 13:44:55
Attachments:
sbcl-backtrace-cleaning.diff
|
dear list, i've attached an updated version of the backtrace cleaning patch. here's a NEWS entry candidate, that hopefully properly describes the patch itself: * imrovement: in debug.lisp backtrace printing, added a *ENTRY-POINT-VERBOSITY* instead of the old *SHOW-ENTRY-POINT-DETAILS*, cleaning of SB-PCL::FAST-METHOD entries, and calling the cleaners on nested entries (e.g. fast-method's inside a xep or similar) i'm not sure about the policy, so please note that this change renames/deletes the previously external SB-DEBUG:*SHOW-ENTRY-POINT-DETAILS*. in the hope it's ok i've updated the documentation accordingly. after applied, (slime) backtraces contain less noise by default (iow, *ENTRY-POINT-VERBOSITY* is 1) which is the most visible with SB-PCL::FAST-METHOD entries. setting it to 3 results in raw backtrace entries and 0 generates the prin1 of the frame entries. i'm not sure about the naming of the global, but i tried to follow the nomenclature used in the documentation. and maybe it should be turned into a more general variable influencing debug verbosity with a more general name? suggestions about the patch (and its possible quality problems) are welcome! on a somewhat related note: this is what i get on both the clean cvs and with this patch included on linux/x86: Finished running tests. Status: Expected failure: debug.impure.lisp / (UNDEFINED-FUNCTION BUG-353) Unexpected success: debug.impure.lisp / (THROW NO-SUCH-TAG) Unexpected success: debug.impure.lisp / (BACKTRACE MISC) Expected failure: external-format.impure.lisp / (CHARACTER-DECODE-LARGE FORCE-END-OF-FILE) Invalid exit status: core.test.sh test failed, expected 104 return code, got 1 comments are welcome, -- attila |
From: Attila L. <att...@gm...> - 2007-04-10 18:38:38
|
> Finished running tests. > Status: > Expected failure: debug.impure.lisp / (UNDEFINED-FUNCTION BUG-353) > Unexpected success: debug.impure.lisp / (THROW NO-SUCH-TAG) > Unexpected success: debug.impure.lisp / (BACKTRACE MISC) > Expected failure: external-format.impure.lisp / (CHARACTER-DECODE-LARGE > FORCE-END-OF-FILE) > Invalid exit status: core.test.sh > test failed, expected 104 return code, got 1 fyi, after some #lisp discussion and a "cvs up" i've checked it again and the core.test.sh error is gone (both with and without my ~/.sbclrc): Finished running tests. Status: Expected failure: debug.impure.lisp / (UNDEFINED-FUNCTION BUG-353) Unexpected success: debug.impure.lisp / (THROW NO-SUCH-TAG) Unexpected success: debug.impure.lisp / (BACKTRACE MISC) Expected failure: external-format.impure.lisp / (CHARACTER-DECODE-LARGE FORCE-END-OF-FILE) ok //apparent success (reached end of run-tests.sh normally) sorry for the false alarm, --- attila |
From: Nikodemus S. <nik...@ra...> - 2007-04-11 06:19:44
|
Attila Lendvai wrote: >> Finished running tests. >> Status: >> Expected failure: debug.impure.lisp / (UNDEFINED-FUNCTION BUG-353) >> Unexpected success: debug.impure.lisp / (THROW NO-SUCH-TAG) >> Unexpected success: debug.impure.lisp / (BACKTRACE MISC) >> Expected failure: external-format.impure.lisp / (CHARACTER-DECODE-LARGE >> FORCE-END-OF-FILE) >> Invalid exit status: core.test.sh >> test failed, expected 104 return code, got 1 > > fyi, after some #lisp discussion and a "cvs up" i've checked it again > and the core.test.sh error is gone (both with and without my > ~/.sbclrc): > > Finished running tests. > Status: > Expected failure: debug.impure.lisp / (UNDEFINED-FUNCTION BUG-353) > Unexpected success: debug.impure.lisp / (THROW NO-SUCH-TAG) > Unexpected success: debug.impure.lisp / (BACKTRACE MISC) > Expected failure: external-format.impure.lisp / (CHARACTER-DECODE-LARGE > FORCE-END-OF-FILE) > ok > //apparent success (reached end of run-tests.sh normally) > > sorry for the false alarm, Could you try one more thing? With the current CVS, and with your old .sbclrc... and if the core.test.sh fails, I would dearly like to know what is in your .sbclrc! Cheers, -- Nikodemus |
From: Juho S. <js...@ik...> - 2007-04-15 23:43:55
|
"Attila Lendvai" <att...@gm...> writes: > suggestions about the patch (and its possible quality problems) are welcome! Thanks. A few comments. Note that the SB-PCL::FAST-METHOD stuff is actually not the same kind of noise that the entry point stuff that's currently being filtered out (SB-C::TL-XEP, etc). Those are basically just internal implementation artifacts. FAST-METHOD on the other hand is part of the actual function name: after evaluating (defmethod baz ((a fixnum) (b float)) that method is actually bound in the function namespace with the name, so it's e.g. possible to do #'(sb-pcl::fast-method baz (fixnum float)). So it's not obvious that we should be mangling the actual name of the function in the backtrace. Second, it's also not obvious to me that the new format is the most readable one possible: 1: ((BAZ (FIXNUM FLOAT)) 1 1.0) Would one of the following maybe look better? 1: ((BAZ FIXNUM FLOAT) 1 1.0) 1: ((METHOD BAZ FIXNUM FLOAT) 1 1.0) > -(defvar *show-entry-point-details* nil) > +(defvar *entry-point-verbosity* 1 Is there any point in making this a multi-state configuration variable? Nobody is ever going to tweak it anyway, so the middle option between raw frames and pretty ones is going to be completely wasted :-) > (defun clean-xep (name args) > - (values (second name) > + (values (or (and (consp name) > + (<= *entry-point-verbosity* 1) > + (second name)) > + name) This'll cause an infinite loop for frames with names like (XEP NIL). Presumably the same also applies to the other clean-* functions that were changed. > -(defun print-frame-call (frame stream &key (verbosity 1) (number nil)) > +(defun print-frame-call (frame stream &key (verbosity *entry-point-verbosity*) > + (number nil)) This conflates two kinds of verbosity. The verbosity that the keyword parameter is referring to is for printing the source location information of the frame for example when you change into it with UP / DOWN in the normal debugger. Deriving it from *E-P-V* means that if somebody were to change *E-P-V*, they'd get funnily formatted source location information in the middle of the stacktrace. -- Juho Snellman |
From: Attila L. <att...@gm...> - 2007-04-18 19:59:18
|
> Note that the SB-PCL::FAST-METHOD stuff is actually not the same kind > of noise that the entry point stuff that's currently being filtered > out (SB-C::TL-XEP, etc). Those are basically just internal > implementation artifacts. FAST-METHOD on the other hand is part of > the actual function name: after evaluating (defmethod baz ((a fixnum) > (b float)) that method is actually bound in the function namespace > with the name, so it's e.g. possible to do #'(sb-pcl::fast-method baz > (fixnum float)). So it's not obvious that we should be mangling the > actual name of the function in the backtrace. let me put it in a different perspective: when using slime it's a bit distracting that the backtrace containes too much noise (from a "user" perspective). i would definately like to be able to switch sbcl/slime in such a mode that all the atrifacts of the implementation is hidden, and preferrably this state should be the default. this noise is especially bad with many levels of methods calls. > Second, it's also not obvious to me that the new format is the most > readable one possible: > > 1: ((BAZ (FIXNUM FLOAT)) 1 1.0) > > Would one of the following maybe look better? > > 2: ((BAZ FIXNUM FLOAT) 1 1.0) > 3: ((METHOD BAZ FIXNUM FLOAT) 1 1.0) now that you mentioned it, i like ((BAZ FIXNUM FLOAT) 1 1.0) the most, but 3 could be used in some more verbose configuration, although the extra paren quite intentionally differentiates simple functions and methods imho. > > -(defvar *show-entry-point-details* nil) > > +(defvar *entry-point-verbosity* 1 > > Is there any point in making this a multi-state configuration > variable? Nobody is ever going to tweak it anyway, so the middle > option between raw frames and pretty ones is going to be completely > wasted :-) see below. > > (defun clean-xep (name args) > > - (values (second name) > > + (values (or (and (consp name) > > + (<= *entry-point-verbosity* 1) > > + (second name)) > > + name) > > This'll cause an infinite loop for frames with names like (XEP NIL). > Presumably the same also applies to the other clean-* functions that > were changed. oops, seems like i'm not completely aware of what may appear as an entry-point. (it misled me that i've used sbck with these changes for the last few months and never experienced any problems) > > -(defun print-frame-call (frame stream &key (verbosity 1) (number nil)) > > +(defun print-frame-call (frame stream &key (verbosity *entry-point-verbosity*) > > + (number nil)) > > This conflates two kinds of verbosity. The verbosity that the keyword > parameter is referring to is for printing the source location > information of the frame for example when you change into it with UP / > DOWN in the normal debugger. Deriving it from *E-P-V* means that if > somebody were to change *E-P-V*, they'd get funnily formatted source > location information in the middle of the stacktrace. hm, that's why i've proposed that maybe there could be a single global verbosity level in sb-debug that controls the verbosity. but thinking again, probably it's better to keep orthogonal boolean vars and/or keywords to control orthogonal things and let the client code control them. thanks for the comments, i'll get back with another proposal in this spirit. -- attila |
From: Juho S. <js...@ik...> - 2007-04-19 07:13:15
|
"Attila Lendvai" <att...@gm...> writes: > oops, seems like i'm not completely aware of what may appear as an > entry-point. (it misled me that i've used sbck with these changes for > the last few months and never experienced any problems) As a terminology note, I think you've got the concepts of entry point and frame backwards :-) A frame is the debugger concept, with there basically being a frame for each function call that hasn't been returned from yet. An entry point is a compiler concept (basically a special kind of function). The reason the variable is named *SHOW-ENTRY-POINT-DETAILS* is that it affects the printing of frames that correspond to calls to entry point functions, but not any other kinds of frames. So the correct name for the new variable you're introducing would be something like *FRAME-VERBOSITY* or *PRINT-FRAME-VERBOSITY*. (Though I don't really think that this should be a variable that users should be tweaking. We should be just providing the sane default. Nor do I understand why *S-E-P-D* is exported, and even documented in the manual.) > > This conflates two kinds of verbosity. The verbosity that the keyword > > parameter is referring to is for printing the source location > > information of the frame for example when you change into it with UP / > > DOWN in the normal debugger. Deriving it from *E-P-V* means that if > > somebody were to change *E-P-V*, they'd get funnily formatted source > > location information in the middle of the stacktrace. > > hm, that's why i've proposed that maybe there could be a single global > verbosity level in sb-debug that controls the verbosity. > > but thinking again, probably it's better to keep orthogonal boolean > vars and/or keywords to control orthogonal things and let the client > code control them. The point is that the former form of verbosity is completely internal to the repl debugger interface, to distinguish between two contexts where frames are printed. It's not something that users can affect in any way, and even if they could, it wouldn't make any sense for them to do so. -- Juho Snellman |
From: Nikodemus S. <nik...@ra...> - 2007-04-19 08:27:27
|
Juho Snellman wrote: > (Though I don't really think that this should be a variable that users > should be tweaking. We should be just providing the sane default. Nor > do I understand why *S-E-P-D* is exported, and even documented in the > manual.) That I can tell. When it was implemented there was desire to have a defined way to see the backtraces the way they used to look (or so I interpreted the consesus at the time.) Cheers, -- Nikodemus |
From: Attila L. <att...@gm...> - 2007-06-17 22:29:14
Attachments:
debug-frame-cleaning.diff
|
i've updated the patch, hopefully according to what was discussed. i've renamed *show-entry-point-details* to *verbosity* which affects the verbosity of the debugger in general. i did not change the documentation yet, because i'm not entirely sure this patch is in accordance with the committers, but if the new sb-debug:*verbosity* (internal) variable is fine, i can send a new patch that includes the update of the docs, too. i've chosen the (method-name :before (type1 type2)) format, because when the qualifiers are present it's confusing not to have the extra parens around the specializers. i hope you'll like the new version, -- attila |
From: Attila L. <att...@gm...> - 2007-07-12 21:27:10
|
> i've updated the patch, hopefully according to what was discussed. > i've renamed *show-entry-point-details* to *verbosity* which affects > the verbosity of the debugger in general. i forgot to mention that i also have this change in slime: (defimplementation print-frame (frame stream) - (sb-debug::print-frame-call frame stream)) + (sb-int:with-sane-io-syntax + (sb-debug::print-frame-call frame stream :verbosity 1))) in the patch i've sent sb-debug::*verbosity* is 2 by default which should probably be 1 and then the :verbosity keyword arg is not needed in slime. -- attila |
From: Attila L. <att...@gm...> - 2008-02-06 10:33:04
Attachments:
improved-frame-cleaning.diff
map-refactor-in-debug.diff
|
dear list, please find attached an updated version of the frame cleaning patch already discussed in this thread (there are only comment changes relative to the one i've sent previously). the second patch factors out internal MAP-BACKTRACE and MAP-FRAME-ARGS functions that came handy when working on the deadlock detector (which is still lingering in some random files here looking for a home together with the PATH-TO-ROOT that helps tracking why stuff is not gc'd. would sb-introspect be an appropriate home for them?) -- attila |
From: Nikodemus S. <nik...@ra...> - 2008-02-06 12:26:28
|
On 2/6/08, Attila Lendvai <att...@gm...> wrote: > dear list, > > please find attached an updated version of the frame cleaning patch > already discussed in this thread (there are only comment changes > relative to the one i've sent previously). > > the second patch factors out internal MAP-BACKTRACE and MAP-FRAME-ARGS > functions that came handy when working on the deadlock detector (which > is still lingering in some random files here looking for a home > together with the PATH-TO-ROOT that helps tracking why stuff is not > gc'd. would sb-introspect be an appropriate home for them?) The refactoring patch looks good -- I'm merged as 1.0.14.21, thank you! I still have some issues with the method frame cleaning: While FAST-METHOD, XEPs, etc, are not of interest of the "average hacker" -- not even most SBCL hackers most of the time, I suspect -- PRINT-FRAME-CALL :VERBOSITY also controls source printing, which is (if you like that sort of thing) a potentially user-friendly feature. Conflating "useless internals noise" and "show the source for this frame" is not good. If also doesn't deal with SB-PCL::SLOW-METHOD frames -- which while rare, also exist. Is this by design? So, I'm thinking that make *VERBOSITY* mean just "internals noise", and add a :PRINT-SOURCE argument to PRINT-FRAME-CALL. Verbosity: 0: #<FRAME> 1: default -- maximally cleaned frames 2: don't clean actual function names (right now just PCL stuff) 3: don't clean entry point details How does this sound? Cheers, -- Nikodemus |
From: Attila L. <att...@gm...> - 2008-02-08 13:47:02
|
> While FAST-METHOD, XEPs, etc, are not of interest of the "average > hacker" -- not even most SBCL hackers most of the time, I suspect -- > PRINT-FRAME-CALL :VERBOSITY also controls source printing, which is > (if you like that sort of thing) a potentially user-friendly feature. > Conflating "useless internals noise" and "show the source for this > frame" is not good. agreed, it was lack of clear sight on my part. i'll introduce a separate :print-source keyword and think through if there are more conflated dimensions. > If also doesn't deal with SB-PCL::SLOW-METHOD frames -- which while > rare, also exist. Is this by design? lack of knowledge on my part and i don't remember seeing them in backtraces. a piece of code that compiles to some frames like that would be helpful. > So, I'm thinking that make *VERBOSITY* mean just "internals noise", > and add a :PRINT-SOURCE argument to PRINT-FRAME-CALL. > > Verbosity: > > 0: #<FRAME> > 1: default -- maximally cleaned frames > 2: don't clean actual function names (right now just PCL stuff) > 3: don't clean entry point details > > How does this sound? sounds good to me. eventually i'll get back with a patch like that and see what others will comment meanwhile. -- attila |
From: Christophe R. <cs...@ca...> - 2008-02-06 12:46:40
|
[ replying to an old message on a point of principle ] "Attila Lendvai" <att...@gm...> writes: > let me put it in a different perspective: when using slime it's a bit > distracting that the backtrace containes too much noise (from a "user" > perspective). i would definately like to be able to switch sbcl/slime > in such a mode that all the atrifacts of the implementation is hidden, > and preferrably this state should be the default. I think I agree with this, but I strongly disagree with the way that you're proposing to implement it: there is nothing more annoying to me than to find that the system has "intelligently" hidden some information which would have been useful to me, which it could have told me about, but which I have no way of reconstructing. I would be entirely happy with some clever slime feature which selectively cleaned the printing of backtraces, preferably togglable on a single keystroke, but I want to register my opposition to removal of potentially significant information. This isn't really addressed just to you, Atilla; I've just read clean-xep/clean-&more-processor/frame-call, and I don't really like it as it currently is in SBCL -- I think that the *show-entry-point-details* default is wrong. (I don't mind reformatting of information, but outright removal to the point that it cannot be deduced is annoying). My preferred way of addressing this would be to work on slime's DEFIMPLEMENTATION PRINT-FRAME for sbcl; I think it would be very reasonable for that to bind *show-entry-point-details* to nil by default, or to do some other frame printing entirely. Best, Christophe |
From: Attila L. <att...@gm...> - 2008-02-08 13:38:46
|
> > let me put it in a different perspective: when using slime it's a bit > > distracting that the backtrace containes too much noise (from a "user" > > perspective). i would definately like to be able to switch sbcl/slime > > in such a mode that all the atrifacts of the implementation is hidden, > > and preferrably this state should be the default. > > I think I agree with this, but I strongly disagree with the way that > you're proposing to implement it: there is nothing more annoying to me > than to find that the system has "intelligently" hidden some > information which would have been useful to me, which it could have > told me about, but which I have no way of reconstructing. > > I would be entirely happy with some clever slime feature which > selectively cleaned the printing of backtraces, preferably togglable > on a single keystroke, but I want to register my opposition to removal > of potentially significant information. This isn't really addressed > just to you, Atilla; I've just read > clean-xep/clean-&more-processor/frame-call, and I don't really like it > as it currently is in SBCL -- I think that the > *show-entry-point-details* default is wrong. (I don't mind > reformatting of information, but outright removal to the point that it > cannot be deduced is annoying). i agree with your points about the wrongness of removal of information, but it all depends on what we consider the "public" interface to the debugger. one can consider MAP-BACKTRACE and MAP-FRAME-ARGS as the canonical interface that provides all the information available, and consider BACKTRACE and PRINT-FRAME-CALL as a dwim-ish interface that renders a string presentation of the backtrace (and FRAME-CALL as a mere internal function). this printing infrastructure has a :verbosity argument, which is when high enough makes printing include all information available. slime could control the printing of the backtrace through some keyword args (e.g. :verbosity :print-source etc) based on that i'd vote against moving this "cleaning" out of SB-DEBUG for two reasons: 1) it's dealing with really internal stuff; locality of dependent code. 2) if we agree that the full backtrace is noisy, then the standard sbcl debugger also needs the frame cleaning code that simplifies it -- attila |
From: Nikodemus S. <nik...@ra...> - 2008-02-08 18:19:54
|
On 2/6/08, Christophe Rhodes <cs...@ca...> wrote: > I think I agree with this, but I strongly disagree with the way that > you're proposing to implement it: there is nothing more annoying to me > than to find that the system has "intelligently" hidden some > information which would have been useful to me, which it could have > told me about, but which I have no way of reconstructing. > > I would be entirely happy with some clever slime feature which > selectively cleaned the printing of backtraces, preferably togglable > on a single keystroke, but I want to register my opposition to removal > of potentially significant information. This isn't really addressed > just to you, Atilla; I've just read > clean-xep/clean-&more-processor/frame-call, and I don't really like it > as it currently is in SBCL -- I think that the > *show-entry-point-details* default is wrong. (I don't mind > reformatting of information, but outright removal to the point that it > cannot be deduced is annoying). I'm not entirely sure what you mean by this. I certainly won't defend the not-very-pretty-implementation, but I don't see how information is being "removed to the point that it cannot be deduced", since the unexpurgated version is there for the asking. > My preferred way of addressing this would be to work on slime's > DEFIMPLEMENTATION PRINT-FRAME for sbcl; I think it would be very > reasonable for that to bind *show-entry-point-details* to nil by > default, or to do some other frame printing entirely. While sbcl-swank.lisp is not very shy when it comes to internals, I would prefer to provide this from within SBCL itself. (Calling SB-DEBUG:DWIM-PRINT-FRAME-CALL instead of PRINT-FRAME-CALL in Swank is fine -- but implementing the frame cleaning inside Swank seems just wrong.) While I don't feel terribly strongly about the default value of *SHOW-ENTRY-POINT-DETAILS*, I do have to say that as long as *DEBUGGER-BEGINNER-HELP-P* is T by default I don't see the point of _not_ cleaning things like this: ((SB-C::HAIRY-ARG-PROCESSOR READ-CHAR) #<SB-SYS:FD-STREAM for "standard input" {117488E9}> NIL #:EOF-OBJECT #<unused argument>) into this: (READ-CHAR #<SB-SYS:FD-STREAM for "standard input" {117488E9}> NIL #:EOF-OBJECT #<unused argument>) I wrote: > Verbosity: > > 0: #<FRAME> > 1: default -- maximally cleaned frames > 2: don't clean actual function names (right now just PCL stuff) > 3: don't clean entry point details but on reflection I think TRT is to add keyword arguments to PRINT-FRAME-CALL -- akin to WRITE. If we in the future come up with new things to toggle in frame printing, cramming them into a single verbosity scale will just be painful. Cheers, -- Nikodemus |
From: Attila L. <att...@gm...> - 2008-03-13 20:03:24
|
> So, I'm thinking that make *VERBOSITY* mean just "internals noise", > and add a :PRINT-SOURCE argument to PRINT-FRAME-CALL. > > Verbosity: > > 0: #<FRAME> > 1: default -- maximally cleaned frames > 2: don't clean actual function names (right now just PCL stuff) > 3: don't clean entry point details i've attached a patch that implements this. swank-sbcl.lisp also needs to be changed after this for better defaults: (defimplementation print-frame (frame stream) (sb-int:with-sane-io-syntax (sb-debug::print-frame-call frame stream :verbosity 1 :print-frame-source nil))) as far as i can see, the patch is in conformance with what was discussed and concluded in this thread, but i'm open for suggestions. i've also attached a separate mini-cleanup for restart-frame. i've added a FIXME noting that someone should fix the way the lambda of the current frame is looked up. what is currently there is wrong and only accidentally work sometimes. somebody with a little more internals knowledge could fix this and then slime could have a working restart frame, which is quite handy sometimes. -- attila |
From: Juho S. <js...@ik...> - 2008-03-13 20:18:59
|
"Attila Lendvai" <att...@gm...> writes: > +(defun restart-frame (frame) > + "Try to restart execution at FRAME. If it's not possible (e.g. due to > +inadequate debug level) then this function returns with (VALUES) ." > + (when (frame-has-debug-tag-p frame) > + ;; FIXME: Getting the first entry in FRAME-CALL-AS-LIST is just plain > + ;; wrong but sometimes accidentally work. Find a way to get hold of the > + ;; function of the current frame and don't forget about fixing Slime either. Getting hold of the function would be the wrong thing to do. The workflow of the people who use RESTART-FRAME is to fix a bug, redefine the function, and then restart the frame. If you use the function object corresponsing to the frame, you'll be restarting the buggy version instead of the fixed version. Sure, this means that only functions visible in the global function namespace are restartable. But that's pretty far from "only sometimes accidentally works". -- Juho Snellman |
From: Attila L. <att...@gm...> - 2008-03-13 20:45:34
|
On 13 Mar 2008 22:19:21 +0200, Juho Snellman <js...@ik...> wrote: > "Attila Lendvai" <att...@gm...> writes: > > +(defun restart-frame (frame) > > + "Try to restart execution at FRAME. If it's not possible (e.g. due to > > +inadequate debug level) then this function returns with (VALUES) ." > > + (when (frame-has-debug-tag-p frame) > > + ;; FIXME: Getting the first entry in FRAME-CALL-AS-LIST is just plain > > + ;; wrong but sometimes accidentally work. Find a way to get hold of the > > + ;; function of the current frame and don't forget about fixing Slime either. > > Getting hold of the function would be the wrong thing to do. The > workflow of the people who use RESTART-FRAME is to fix a bug, redefine > the function, and then restart the frame. If you use the function > object corresponsing to the frame, you'll be restarting the buggy > version instead of the fixed version. i may be off here, but i've always wanted to restart at older frames than where i was fixing a problem. but as i haven't used it much yet (because i've hardly seen it working from slime), this is fwiw... > Sure, this means that only functions visible in the global function > namespace are restartable. But that's pretty far from "only sometimes > accidentally works". well, before my frame cleaning changes, frame-call-as-list used to return all kind of things as car of the result, which was often something else then a symbol naming a function. but your point is completely valid. in the lights of this i suggest to keep this code, but then add a :verbosity 1 to the call to frame-call-as-list and if that fails to return anything usable, then try to look up the function of the requested frame. that way it would pick up redefined functions, but also work for frames for which frame-call-as-list does not return a valid function name. -- attila |
From: Juho S. <js...@ik...> - 2008-03-13 20:52:42
|
"Attila Lendvai" <att...@gm...> writes: > well, before my frame cleaning changes, frame-call-as-list used to > return all kind of things as car of the result, which was often > something else then a symbol naming a function. We specifically don't want just *symbols* naming functions. CL-USER> (defmethod foo :before ((a (eql #.*a*))) 1) #<STANDARD-METHOD FOO :BEFORE ((EQL #(1 2 3))) {B786F11}> CL-USER> (fdefinition '(sb-pcl::fast-method foo :before ((eql #.*a*)))) #<FUNCTION (SB-PCL::FAST-METHOD FOO :BEFORE ...)> -- Juho Snellman |
From: Attila L. <att...@gm...> - 2008-03-13 21:08:15
|
> > well, before my frame cleaning changes, frame-call-as-list used to > > return all kind of things as car of the result, which was often > > something else then a symbol naming a function. > > We specifically don't want just *symbols* naming functions. > > CL-USER> (defmethod foo :before ((a (eql #.*a*))) 1) > #<STANDARD-METHOD FOO :BEFORE ((EQL #(1 2 3))) {B786F11}> > CL-USER> (fdefinition '(sb-pcl::fast-method foo :before ((eql #.*a*)))) > #<FUNCTION (SB-PCL::FAST-METHOD FOO :BEFORE ...)> ok, make it (frame-call-as-list frame :verbosity 2) then, which will clean xep and things like that but won't touch actual function names (these are sb!pcl::fast-method and sb!pcl::slow-method currently). btw, doesn't grabbing (fdefinition 'foo) in this case do the right thing, only a generic dispatch slower? but either way, going through the symbol lookup first is a good thing, but it'll need some more error handling then. if somebody could paste how to look up the lambda for a frame, then i can offer to debug and resend this patch with the possible required slime changes. -- attila |
From: Attila L. <att...@gm...> - 2008-08-04 14:39:56
|
please find an updated version of the patches attached. i'm using these for a long time now, and i don't know of anything breaking due to this. the differences to the previous versions of this patch are relatively small: - clarify restart-frame's comment based on Juho's mail - fix some tests that were broken due to the restart-frame changes - use the &key ((:verbosity *verbosity*) *verbosity*) pattern for rebinding *verbosity* (mimicing the implementation of the print-.* functions) -- attila ps: updating slime after applying this patch is not necessary anymore, because the default value of *verbosity* in this version of the patch is ok for slime, too. |
From: Attila L. <att...@gm...> - 2009-08-20 15:09:22
|
please find the fresh versions of the two patches attached. i've changed the comment in restart-frame incorporating Juho's remarks, made some tiny changes, and rebased on sbcl HEAD. any chance of incorporating this into the official repo? anything i can amend to make it acceptable? -- attila |
From: Tobias C. R. <tc...@fr...> - 2009-09-15 17:37:25
|
Attila Lendvai <att...@gm...> writes: > please find the fresh versions of the two patches attached. > > i've changed the comment in restart-frame incorporating Juho's > remarks, made some tiny changes, and rebased on sbcl HEAD. > > any chance of incorporating this into the official repo? anything i > can amend to make it acceptable? I just tried it because I'd really like to see something along these lines be committed. CLOS-heavy backtraces tend to be a chore to digest. What is the reason that (SB-PCL::FAST-METHOD FOO:BAR (FUNCTION)) is printed as (FOO:BAR (FUNCTION)) and not simply FOO:BAR ? What useful information does the CDDR of a FAST-METHOD name ever provide? -T. |
From: Attila L. <att...@gm...> - 2009-09-18 06:39:05
Attachments:
improved-frame-cleaning.diff.gz
|
> What is the reason that > > (SB-PCL::FAST-METHOD FOO:BAR (FUNCTION)) > > is printed as > > (FOO:BAR (FUNCTION)) > > and not simply > > FOO:BAR ? > > What useful information does the CDDR of a FAST-METHOD name ever > provide? well, one can argue: 1) that form points out that it's a method not a function 2) shows which specific method is used there although 2) is a bit redundant with 'v' in sldb (which doesn't work after C-c C-c'ing anything with a custom reader, but that's a different issue...) so, after quickly trying out your suggestion, i've ended up with the modified patch that does what you proposed on (<= *verbosity* 1) and the previous behavior when it's 2. this way both behavior is available with a more sensible default. -- attila ps: there's a tailor converted darcs2 repo of sbcl with all our modifications (including this updated patch) at http://dwim.hu/darcs/ or alternatively at: http://common-lisp.net/~alendvai/darcs/sbcl/ |