Subject: [clisp-list] SBCL build, MAKE-LOAD-FORM, buglets

From: Christophe Rhodes <[email-masked]>
Date: Thu Apr 10 05:28:04 2003

Hi,

Firstly, let me just thank you for being able to announce another
milestone in the ability of clisp to act as host Lisp for SBCL's
build.  Compilation of the 289 files now proceeds to completion,
before the linking ("genesis") phase dies shortly after startup with
an error that I don't yet understand.

To get this far, I had to turn off complex uses of the pretty printer,
I'm afraid; it's all too easy to trigger assertion errors or crash the
system (and I haven't yet had any luck in distilling this to a small
test case, more's the pity).  Apart from fixing some portability
issues in sbcl's code base, there was one other major problem that I
think is an issue in CLISP; I've worked around it for now, but you
should know about it.

At the toplevel, do
  (defstruct foo a)
  (defmethod make-load-form ((x foo) &optional env)
    (make-load-form-saving-slots x :environment env))
and put in a file, say foo.lisp:
  (defvar *foo* '#S(FOO :A BAR))

Compile and load the file, and observe that *FOO* is correctly
initialized to a FOO structure whose A slot contains the symbol BAR.
So far, so good.  Now, again at the toplevel, continuing the session,
do:
  (makunbound '*foo*)
  (defconstant bar 1)
and recompile and reload foo.lisp.  Now I observe that *FOO* is bound
to #S(FOO :A 1) and not to #S(FOO :A BAR), which I believe is
incorrect behaviour.

Cheers,

Christophe
-- 
http://www-jcsu.jesus.cam.ac.uk/~csr21/       +44 1223 510 299/+44 7729 383 757
(set-pprint-dispatch 'number (lambda (s o) (declare (special b)) (format s b)))
(defvar b "~&Just another Lisp hacker~%")    (pprint #36rJesusCollegeCambridge)

---

From: Sam Steingold <[email-masked]>
Date: Wed Apr 16 16:10:17 2003

> * In message <[email-masked]>
> * On the subject of "[clisp-list] SBCL build, MAKE-LOAD-FORM, buglets"
> * Sent on Thu, 10 Apr 2003 13:26:49 +0100
> * Honorable Christophe Rhodes <[email-masked]> writes:
>
> At the toplevel, do
>   (defstruct foo a)

please try the appended patch.

-- 
Sam Steingold (http://www.podval.org/~sds) running RedHat8 GNU/Linux
<http://www.camera.org> <http://www.iris.org.il> <http://www.memri.org/>
<http://www.mideasttruth.com/> <http://www.palestine-central.com/links.html>
Flying is not dangerous; crashing is.

--- loadform.lisp.~1.4.~	2001-08-20 16:52:13.000000000 -0400
+++ loadform.lisp	2003-04-16 18:57:18.000000000 -0400
@@ -31,34 +31,13 @@
   (:method ((object standard-object) &optional environment)
     (make-load-form-saving-slots object :environment environment)))
 
-(defun mlf-unquote (var form)
-  ;; replace '(... var ...) with `(... ,var ...)
-  ;; (setq v 10)
-  ;; (mlf-unquote 'v ''(1 (2 (b c) v d (a)))) ==>
-  ;; (LIST 1 (LIST 2 '(B C) V 'D '(A)))
-  (cond ((atom form) form)
-        ((eq (car form) 'quote)
-         (labels ((find-var (tree)
-                    (if (atom tree) (eq var tree)
-                        (or (find-var (car tree)) (find-var (cdr tree)))))
-                  (unquote (form)
-                    (if (consp form)
-                        (if (find-var form)
-                            (cons 'list (mapcar #'unquote form))
-                            (list 'quote form))
-                        (if (eq form var) form
-                            (if (constantp form) form (list 'quote form))))))
-           (unquote (cadr form))))
-        (t (cons (mlf-unquote var (car form))
-                 (mlf-unquote var (cdr form))))))
-
 (defun mlf-init-function (object)
   (multiple-value-bind (cre-form ini-form) (make-load-form object)
     (if ini-form
         (let ((var (gensym)))
           `(lambda ()
             (let ((,var ,cre-form))
-              ,(mlf-unquote var (nsubst var object ini-form))
+              ,(nsubst var object ini-form)
               ,var)))
         `(lambda () ,cre-form))))

---

From: Christophe Rhodes <[email-masked]>
Date: Sat Apr 19 08:04:06 2003

> please try the appended patch.

[ and on clisp-devel CVS digest: ]
> Log Message:
> (mlf-unquote): removed (use SUBLIS instead)

Oh, damn, I was about to tell you that the patch you sent me didn't
work (specifically, caused failure modes in sbcl's build) :-/

However, let me mix in some good news with the bad: with a lightly
frobbed version of MLF-UNQUOTE (and nothing else unchanged in
loadform.lisp) and a fix to defmacro.lisp, the version of clisp I have
now successfully compiles SBCL.  (well, more or less; there are some
residual bugs that I haven't yet sorted out, but it's basically
there).

So.  Here's what I have for MLF-UNQUOTE:

(defun mlf-unquote (var form)
  ;; replace '(... var ...) with `(... ,var ...)
  ;; (setq v 10)
  ;; (mlf-unquote 'v ''(1 (2 (b c) v d (a)))) ==>
  ;; (LIST 1 (LIST 2 '(B C) V 'D '(A)))
  (cond ((atom form) form)
        ((eq (car form) 'quote)
         (labels ((find-var (tree)
                    (if (atom tree) (eq var tree)
                        (or (find-var (car tree)) (find-var (cdr tree)))))
                  (unquote (form)
                    (if (consp form)
                        (if (find-var form)
                            (cons 'list (mapcar #'unquote form))
                            (list 'quote form))
                        (if (eq form var) form
                            (list 'quote form)))))
           (unquote (cadr form))))
        (t (cons (mlf-unquote var (car form))
                 (mlf-unquote var (cdr form))))))

My reasoning is that the call to CONSTANTP looked like it really
wanted to be a call to a putative SELF-EVALUATING-P, and as such was
performing a very light optimization.  I don't know if what I have now
is equivalent to what you have (in a sense, I'm scared to cvs upgrade
as I chase the residual bugs :-) but that's what I have that seems to
work.  The version with the patch you sent me blows up with an error
of the form 
  *** SYSTEM:%STRUCTURE-REF:  #:G1234 is not of type LAYOUT
(I forget the exact wording, but it was definitely a manifestation of
insufficient evaluation at some stage).

Also, please find attached a hacky patch to defmacro.lisp.  The
symptom it cures is:
  (defmacro foo (&key (b t)) (if b 'c 'd))
  (macroexpand '(foo)) -> C
  (macroexpand '(foo :b nil)) -> C ; should be D
but you will want to replace the :%magic keyword with a fresh cons or
a gensym or some other object that can't appear in an argument list.
I hope that the general idea is clear, anyway.  Oh, and I may not be
getting it right with multiple evaluation -- that may be something
else to check.

Index: src/defmacro.lisp
===================================================================
RCS file: /cvsroot/clisp/clisp/src/defmacro.lisp,v
retrieving revision 1.13
diff -u -r1.13 defmacro.lisp
--- src/defmacro.lisp	25 Mar 2003 20:04:19 -0000	1.13
+++ src/defmacro.lisp	19 Apr 2003 09:07:25 -0000
@@ -175,9 +175,12 @@
                             (cons `(,(car next)
                                      ;; the default value should not be
                                      ;; evaluated unless it is actually used
-                                     (or (GETF ,restvar ,kw)
-                                         ,@(when svar `((setq ,svar nil)))
-                                         ,(cadr next)))
+                                     (if (eq (GETF ,restvar ,kw :%magic)
+					     :%magic)
+					 (progn
+					  ,@(when svar `((setq ,svar nil)))
+					  ,(cadr next))
+					 (getf ,restvar ,kw)))
                                   %let-list))
                       (setq kwlist (cons kw kwlist)))
                      ((not (and (consp (car next)) (symbolp (caar next)) (consp (cdar next))))
@@ -190,9 +193,12 @@
                         (setq %let-list (cons `(,svar t) %let-list)))
                       (setq %let-list
                             (cons `(,(cadar next)
-                                     (or (GETF ,restvar ,kw)
-                                         ,@(when svar `((setq ,svar nil)))
-                                         ,(cadr next)))
+                                     (if (eq (GETF ,restvar ,kw :%magic)
+					     :%magic)
+					 (progn
+					   ,@(when svar `((setq ,svar nil)))
+					   ,(cadr next))
+					 (getf ,restvar ,kw)))
                                   %let-list))
                       (setq kwlist (cons kw kwlist)))
                      (t ; subform
@@ -206,9 +212,12 @@
                       (when svar
                         (setq %let-list (cons `(,svar t) %let-list)))
                       (setq %let-list
-                            (cons `(,g (or (GETF ,restvar ,kw)
-                                           ,@(when svar `((setq ,svar nil)))
-                                           ,(cadr next)))
+                            (cons `(,g (if (eq (getf ,restvar ,kw :%magic)
+					       :%magic)
+					   (progn
+					     ,@(when svar `((setq ,svar nil)))
+					     ,(cadr next))
+					   (getf ,restvar ,kw)))
                                   %let-list))
                       (setq kwlist (cons kw kwlist))
                       (let ((%min-args 0) (%arg-count 0) (%restp nil)

Many thanks,

Christophe

---

From: Sam Steingold <[email-masked]>
Date: Sat Apr 19 14:16:49 2003

> * In message <[email-masked]>
> * On the subject of "Re: [clisp-list] SBCL build, MAKE-LOAD-FORM, buglets"
> * Sent on Sat, 19 Apr 2003 16:02:32 +0100
> I don't know if what I have now is equivalent to what you have (in a
> sense, I'm scared to cvs upgrade as I chase the residual bugs :-) but
> that's what I have that seems to work.  The version with the patch you
> sent me blows up with an error of the form

what I have in the CVS now is different from what I sent you.
it should work unless your MAKE-LOAD-FORM method does something really
weird, like '(... <object> ...).

please do try it, and if it does not work, send me the MAKE-LOAD-FORM
method on which it breaks.

>   (macroexpand '(foo :b nil)) -> C ; should be D

I introduced this bug while fixing your previous bug.
should be fixed now.

-- 
Sam Steingold (http://www.podval.org/~sds) running RedHat8 GNU/Linux
<http://www.camera.org> <http://www.iris.org.il> <http://www.memri.org/>
<http://www.mideasttruth.com/> <http://www.palestine-central.com/links.html>
The only substitute for good manners is fast reflexes.

---

From: Christophe Rhodes <[email-masked]>
Date: Sun Apr 20 02:00:02 2003

> I introduced this bug while fixing your previous bug.

Heh, thanks.  OK, I'll try to give it a spin later this weekend and
will report back as soon as I can :-)

Cheers,

Christophe

---

From: Christophe Rhodes <[email-masked]>
Date: Sun Apr 20 08:07:05 2003

> will report back as soon as I can :-)

I've done this, and the build has just gone to completion.

Cheers,

Christophe