From: Rob M. <ra...@ri...> - 2005-12-30 15:25:15
|
If SB-KERNEL:FLOAT-WAIT doesn't appear explicitly in the source code being compiled, then there shouldn't be an unreachable code note, at least not for that particular source location. This test is somewhat hueristic, but the idea is that insofar as possible, dead code introduced by macros should not cause a note. In this case, the macro with-timeout is definitely going to cause dead code notes, but the top-level source context of the note should be something that explicitly appears in the orginal source. i.m.o. it would be better to change with-timeout to use FLET so as not to duplicate the body. Rob |
From: Travis C. <tr...@cr...> - 2005-12-30 17:53:55
Attachments:
with-timeout-change.diff
|
Rob MacLachlan wrote: > This test is somewhat hueristic, but the idea is that insofar as > possible, dead code introduced by macros should not cause a note. That is good to know. I was unaware that there was any magic that goes on here. As a rule, I treat the note as constructive, even with code generated by macros. Usually, if a macro is causing code to be deleted in my application, the macro could be written in a better fashion. > If SB-KERNEL:FLOAT-WAIT doesn't appear explicitly in the source code > being compiled, then there shouldn't be an unreachable code note, at > least not for that particular source location. This does seem to violate the principle of least surprise to the developer. The simplest case of this at work is the code (on x86): (handler-case (princ "foobar") (error (e) e)) which produces: ; in: LAMBDA NIL ; (SB-KERNEL:FLOAT-WAIT) ; ; note: deleting unreachable code ; ; compilation unit finished ; printed 1 note foobar "foobar" This is not a result of the with-timeout code. The expansion of handler-bind creates a call to sb-kernel:float-wait that occurs _after_ an unconditional call to return-from (generated by handler-case). This creates hopelessly stranded code. Both handler-case and handler-bind add a #!+x86 (float-wait) to be evaluated after the body forms. In the case of handler-bind, this (float-wait) occurs after the non-local exit made after the form passed to handler-case is evaluated. To clarify, the full expansion of the above is: (block #:g1040 (let ((#:g1041 nil)) (declare (ignorable #:g1041)) (tagbody (let ((sb-kernel:*handler-clusters* (cons (list (cons 'error (lambda (sb-impl::temp) (setq #:g1041 sb-impl::temp) (go #:g1042)))) sb-kernel:*handler-clusters*))) (multiple-value-prog1 (progn (return-from #:g1040 (multiple-value-prog1 (princ "foobar") (sb-kernel:float-wait)))) (sb-kernel:float-wait))) ;;; <<--- *deleted* *code* #:g1042 (return-from #:g1040 (let ((e #:g1041)) e))))) The last float-wait is the one that is nailing us. I would be greatly interested if someone here would explain what exactly float-wait does and why it is necessary in certain code locations for x86 (and not x86_64?). > In this case, the macro with-timeout is definitely going to cause > dead code notes, but the top-level source context of the note should > be something that explicitly appears in the orginal source. I certainly agree. I could imagine these notes being immensely confusing and discomforting to someone new to SBCL. I believe both issues raised here should be resolved. > i.m.o. it would be better to change with-timeout to use FLET so as > not to duplicate the body. Even if the body is not duplicated in the expansion, the deleted call to the flet bound function would be noted by SBCL. This note, since it would refer to code not in the original source, would be even less clear. In my opinion, with-timeout should simply timeout immediately if passed a non-positive value for `expires`. If I have code that has the possibility of passing 0 or -1 to with-timeout, I would be expecting it to signal a sb-ext:timeout condition. A simple patch to achieve this is attached. In fact, allowing code to rely on this artifact (passing 0 or negative numbers to with-timeout to prevent timeout checking) just feels dirty ;) As far as handler-case / handler-bind goes, I would like to better understand (sb-kernel:float-wait) before proposing a patch. Cheers, -- Travis |
From: Christophe R. <cs...@ca...> - 2005-12-30 18:44:38
|
Travis Cross <tr...@cr...> writes: > (handler-case (princ "foobar") > (error (e) e)) > > which produces: > > ; in: LAMBDA NIL > ; (SB-KERNEL:FLOAT-WAIT) > ; > ; note: deleting unreachable code > ; > ; compilation unit finished > ; printed 1 note > foobar > "foobar" Last time I tried looking at this (October 2003, in case people want to search the mail archives) I failed to make headway. The FLOAT-WAIT is needed in order to ensure precise floating point exception reporting. (It might be necessary on the x86-64, I don't know). On the x86 architecture, a floating point exception is reported by the CPU on the /next/ floating point instruction, not the one causing the exception; so the float-wait is necessary to ensure that any floating point exceptions that are going to be signalled have been signalled. As for why it's showing a deletion note, I'm at least slightly baffled, because you will find that if you redefine handler-case at the repl (unchanged; you might need to load src/cold/chill.lisp first) the code deletion messages go away. The last thing I remember trying was copying the macro expansion tree, on the assumption that it might be an artifact of constant sharing between handler-case's and handler-bind's macroexpander code confusing the compiler note heuristics. According to <http://www.caddr.com/macho/archives/sbcl-devel/2003-10/2660.html>, this didn't help. Cheers, Christophe |
From: Rob M. <ra...@ri...> - 2005-12-30 19:53:10
|
Travis Cross wrote: > >>i.m.o. it would be better to change with-timeout to use FLET so as >>not to duplicate the body. > > > Even if the body is not duplicated in the expansion, the deleted call to > the flet bound function would be noted by SBCL. This note, since it > would refer to code not in the original source, would be even less clear. I haven't tried it just now, but I recall doing this in the past in similar cases for this reason. Because the call to the FLET function is not in the original source, there should be no note. The code may be different in sbcl, but in cmucl we only give a note for a list form if that form is "original source", which means that it is EQ to part of the form originally returned by the reader. But the note we're getting doesn't make any sense either. It seems like the source path is getting messed up somehow. This is the CMUCL code: (defun note-block-deletion (block) (let ((home (block-home-lambda block))) (unless (eq (functional-kind home) :deleted) (do-nodes (node cont block) (let* ((path (node-source-path node)) (first (first path))) (when (or (eq first 'original-source-start) (and (atom first) ... atom hueristics ..)) (unless (return-p node) (let ((*compiler-error-context* node)) (compiler-note "Deleting unreachable code."))) (return)))))) (undefined-value)) When this approach fails is mainly with macros like duplicate code, as though one copy may be deleted, it does not reach the certain futility required to justify a note. Due to the hueristics for atoms it can also cause spurious warnings for things such as references to local variables that happen to have the same name as an argument variable. Changing the macro semantics to avoid the need for dulication would also work. Rob |