Hello,
While trying to compile some code that contained the #+#. idiom I came across a bug in SLIME. Here's a minimal test case:
(defun foo () #+#.'(:and) (/ 1 0))
And some variations:
(defun foo () #+#.'(:and) t (/ 1 0))
(defun foo () (let (unused) #+#.'(:and) t))
Trying to compile these examples through, e.g., slime-compile-file or slime-compile-defun results in a cryptic error:
unknown operator in feature expression: (#:|#.1|). [Condition of type SIMPLE-ERROR]
What these three examples have in common is that a compiler note is generated and the offending form contains #+#. which makes SWANK-BACKEND::LOCATE-COMPILER-NOTE choke. What happens is that READ-AND-RECORD-SOURCE-MAP overrides #. and makes it return a gensym, #:|#.1| in this particular example, which is then passed to #+ and causes the error above.
SUPPRESS-SHARP-DOT sets things up so that #. returns a gensym instead of NIL in order to "avoid multiple entries for nil at toplevel in the source-map", according to the ChangeLog. Even if we ignore that problem, having #. return NIL in this case, while it would dodge the error, it wouldn't find or properly annotate the offending (/ 1 0) form since #+cl:nil usually suppresses forms.
The attached patch gets rid of SUPPRESS-SHARP-DOT. That not only fixes my original problem but also makes the following example work better than it before, since the (/ 1 0) form is now annotated:
#.'(defun foo () (/ 1 0))
I believe that was the case the ChangeLog entry I quoted was referring to. However, I have the feeling that there might be some good reason why #. was suppressed in the first place. Any ideas?
(BTW, it took me a while to figure out this bug was coming from within SLIME itself, and it might have taken even longer if someone hadn't noticed that compiling the software in question without SLIME worked without problem. Perhaps it would be helpful to somehow report internal errors as such?)
* Luis Oliveira [2008-01-22 01:33+0100] writes:
[...]
SUPPRESS-SHARP-DOT sets things up so that #. returns a gensym instead of NIL in order to "avoid multiple entries for nil at toplevel in the source-map", according to the ChangeLog. Even if we ignore that problem, having #. return NIL in this case, while it would dodge the error, it wouldn't find or properly annotate the offending (/ 1 0) form since #+cl:nil usually suppresses forms.
That's true.
The attached patch gets rid of SUPPRESS-SHARP-DOT. That not only fixes my original problem but also makes the following example work better than it before, since the (/ 1 0) form is now annotated:
#.'(defun foo () (/ 1 0))
True, but it seems a bit unusual to use #. for inserting big quoted forms.
I believe that was the case the ChangeLog entry I quoted was referring to. However, I have the feeling that there might be some good reason why #. was suppressed in the first place. Any ideas?
This thread is probably also related: http://thread.gmane.org/gmane.lisp.slime.devel/4322
I think it boils down to the question whether *read-eval* should be enabled or not.
What is important, is that we can read the entire toplevel form without raising errors and that the "shape" of the form is more or less correct.
#. could have arbitrary side-effects. The most common case is probably that #. can't read or evaluate the expression and signals an error. Think of reading SBCL source code, which often refers to packages or constants which no longer exist at runtime. Also, in most cases the actual expression returned by #. is irrelevant because we must skip over it anyway (#+#. is an exception). For the lookup in the source-map it doesn't matter much if we intern symbols in the wrong package (unnecessary interning is annoying, but not fatal). But the proper package is usually important for #., since the expression must be evaluated.
We could try both, first reading with *read-eval* enabled and if that fails retry with *read-eval* disabled. (Let's assume we can rewind the underlying stream.) But that's somewhat complicated and isn't a 100% solution either.
Your proposal, let *read-eval* be whatever it was, sounds better. I will apply the patch.
(BTW, it took me a while to figure out this bug was coming from within SLIME itself, and it might have taken even longer if someone hadn't noticed that compiling the software in question without SLIME worked without problem. Perhaps it would be helpful to somehow report internal errors as such?)
That sounds like a good idea. We have to think about how to do it without cluttering the code to much.
Helmut.
-
Helmut Eller -
Luis Oliveira