From: Jack
Subject: Bug hunting/lexical confusion
Date: 
Message-ID: <1130623234.096442.90770@g43g2000cwa.googlegroups.com>
A project of mine needs a zip file reader. I found the Common Lisp Zip
Library (http://common-lisp.net/project/zip/ ) and decided to test it
for my purposes. I ran preliminary tests (i.e., compiling and loading
the library) in two different environments, Clisp 2.35 and SBCL 0.9.5
(SBCL + XEmacs + Slime), with varying results. (Despite its power,
(X)Emacs is SO F'IN CLUMSY--I CAN'T STAND IT! I'm STILL looking for a
more comfortable editor [for Linux] with similar features.) An issue
causing a failure on loading in Clisp raised some confusion about the
standard vs. implementation. I haven't found relevant details in the
Hyperspec, so I wondered if anyone might have the answer.

First, since the zip library hasn't yet released support for Clisp and
I hadn't yet seen the patch at the project's message archive, I hacked
in some #clisp-dependent features, threw together something for the
gray support in a file named clisp.lisp and called (asdf:operate
'load-op 'zip). It successfully compiled zip.lisp, but upon loading
zip.fas, CLISP choked and kicked into the debugger giving the
following:

...
;; Loading file /home/stranger/oss/lisp/zip/zip.fas ...
*** - EVAL: variable #:G9428 has no value
The following restarts are available:
...
ABORT          :R5      ABORT

Of course, the name identifies a gensym. I suspected the error spawned
from a specific macro, the only use of gensym I could find in the
source, and the position of the symbol in lisp.fas confirmed my
suspicion.

The following code raises the error:

(defmacro define-record (constructor (&key (length (gensym))) &rest
fields)
  `(progn
     (defconstant ,length
	 ,(loop
	      for (nil type) in fields
	      sum (ecase type (:int 4) (:short 2))))
     (defun ,constructor (&optional s)
	; BAD: ,length expands to #Gxxxx, instead of the actual value.
	; The CLISP 2.35 compiler doesn't complain, but the loader chokes on
the
	; uninterned symbol.
       (let ((bytes (make-byte-array ,length)))
	 (when s
           (read-sequence bytes s))
	 bytes))
     ,@(loop
	   for (name type) in fields
	   for offset = 0 then (+ offset length)
	   for length = (ecase type (:int 4) (:short 2))
	   for reader = (ecase type (:int 'get-int) (:short 'get-short))
	   unless (eq name :dummy)
	   append `((defun ,name (r)
                      (,reader r ,offset))
                    (defun (setf ,name) (newval r)
                      (setf (,reader r ,offset) newval))))))

...

(define-record make-directory-entry ()
  (cd/signature :int)
  (cd/version-made-by :short)
  (cd/version-needed-to-extract :short)
  (cd/flags :short)
  (cd/method :short)
  (cd/time :short)
  (cd/date :short)
  (cd/crc :int)
  (cd/compressed-size :int)
  (cd/size :int)
  (cd/name-length :short)
  (cd/extra-length :short)
  (cd/comment-length :short)
  (cd/disc-number :short)
  (cd/internal-attributes :short)
  (cd/external-attributes :int)
  (cd/offset :int))

...

zip.fas shows 3 unique gensyms from 3 calls to define-record without
field names. If not for #G948, the loader would have choked on the next
use of the macro without the :length. If not for the second, it would
have choked on the third.

The macro expands as follows:

[7]> (pprint (macroexpand '(define-record make-directory-entry ()
(end/signature :int) (end/this-disc :short) (end/central-directory-disc
:short) ...)))

(PROGN (DEFCONSTANT #:G9428 46)
 (DEFUN MAKE-DIRECTORY-ENTRY (&OPTIONAL S)
  (LET ((BYTES (MAKE-BYTE-ARRAY #:G9428))) (WHEN S (READ-SEQUENCE BYTES
S))
   BYTES))
 (DEFUN END/SIGNATURE (R) (GET-INT R 0))
 (DEFUN (SETF END/SIGNATURE) (NEWVAL R) (SETF (GET-INT R 0) NEWVAL))
 (DEFUN END/THIS-DISC (R) (GET-SHORT R 4)) ...)

Does it make any sense to define a constant gensym, if you can't
evaluate it in any package?

At first, I assumed a bug in the macro definition and wondered how
anyone successfully used the zip library on any implementation
whatsoever.

I considered modifying the macro to avoid defining a constant defsym in
calls to the macro without the :length. In the following first draft I
ignored the capture issue:

(defmacro define-record (constructor (&key length-name) &rest fields)
  (let ((length (loop for (nil type) in fields
		    sum (ecase type (:int 4) (:short 2)))))
    `(progn
       ,@(when length-name (list `(defconstant ,length-name ,length)))
       (defun ,constructor (&optional s)
	 (let ((bytes (make-byte-array ,length)))
	   (when s (read-sequence bytes s))
	   bytes))
       ,@(loop
	 for (name type) in fields
	 for offset = 0 then (+ offset length)
	 for length = (ecase type (:int 4) (:short 2))
	 for reader = (ecase type (:int 'get-int) (:short 'get-short))
	 unless (eq name :dummy)
	 append `((defun ,name (r)
		    (,reader r ,offset))
		  (defun (setf , name) (newval r)
		    (setf (,reader r ,offset) newval)))))))

(Notice that I changed the key name from :length to :length-name,
because the name more closely reflects the purpose of the key. I
suppose it comes down to personal preference, but I only needed to
change it once outside of the macro definition, in make-end-header.)

The improved macro doesn't define a constant if the call to
(define-record) doesn't have a :length.

SBCL 0.9.5 (which I just started using and in which I had previously
neglected to test the original) expands the revised macro for
make-end-header as follows:

CL-USER> (pprint (macroexpand '(define-record make-end-header
(:length-name +end-header-length+)
  (end/signature :int)
  (end/this-disc :short)
  (end/central-directory-disc :short)
  (end/disc-files :short)
  (end/total-files :short)
  (end/central-directory-size :int)
  (end/central-directory-offset :int)
  (end/comment-length :short))))

(PROGN
 (DEFCONSTANT +END-HEADER-LENGTH+ 22)
 (DEFUN MAKE-END-HEADER (&OPTIONAL S)
   (LET ((BYTES (MAKE-BYTE-ARRAY 22)))
     (WHEN S (READ-SEQUENCE BYTES S))
     BYTES))
 (DEFUN END/SIGNATURE (R) (GET-INT R 0))
 (DEFUN (SETF END/SIGNATURE) (NEWVAL R) (SETF (GET-INT R 0) NEWVAL))
...)

For the definition which caused all of the trouble (without the field
name), SBCL gave the following:

CL-USER> (pprint (macroexpand '(define-record make-directory-entry ()
  (cd/signature :int)
  (cd/version-made-by :short) ...)))

(PROGN
 (DEFUN MAKE-DIRECTORY-ENTRY (&OPTIONAL S)
   (LET ((BYTES (MAKE-BYTE-ARRAY 46)))
     (WHEN S (READ-SEQUENCE BYTES S))
     BYTES))
 (DEFUN CD/SIGNATURE (R) (GET-INT R 0))
 (DEFUN (SETF CD/SIGNATURE) (NEWVAL R) (SETF (GET-INT R 0) NEWVAL))
...)

I realized that I would need to address the capture issue, which would
probably require gensyms. I wasn't sure how I would implement it
without running into the original problem. Lack of sleep forced me to
put it off until another day.

This morning, I decided to look at it from a different angle: Was it an
implementation issue?

I loaded the zip.asd in SBCL, without a complaint.

In XEmacs/Slime, I evaluated the following:

(pprint (macroexpand '(define-record make-directory-entry ()
  (cd/signature :int)
  (cd/version-made-by :short) ...)))

This gave me the following in the REPL:

(PROGN
 (DEFCONSTANT #:G4307 46)
 (DEFUN MAKE-DIRECTORY-ENTRY (&OPTIONAL S)
   (LET ((BYTES (MAKE-BYTE-ARRAY #:G4307)))
     (WHEN S (READ-SEQUENCE BYTES S))
     BYTES))
 (DEFUN CD/SIGNATURE (R) (GET-INT R 0))
 (DEFUN (SETF CD/SIGNATURE) (NEWVAL R) (SETF (GET-INT R 0) NEWVAL))
 (DEFUN CD/VERSION-MADE-BY (R) (GET-SHORT R 4))
 (DEFUN (SETF CD/VERSION-MADE-BY) (NEWVAL R) (SETF (GET-SHORT R 4)
NEWVAL)) ...)

Except for the gensym name, the expansion looks identical to the same
expansion in Clisp.

I guess I don't quite understand how (DEFUN MAKE-DIRECTORY-ENTRY ...),
a top-level form, could refer to a gensym defined in another top-level
form, since a gensym is by definition uninterned. Does  (DEFCONSTANT
#:G4307) bind #:G4307 within the lexical environment of the PROGN,
thereby making it available within (DEFUN MAKE-DIRECTORY-ENTRY ...)?
(Of course, the macro refers to the gensym by a different name, but
Clisp wants to evalate the #:Gxxx and says it can't.)

Does the failure in Clisp indicate a bug in Clisp 2.35? [Is this
related to CLHS 3.1.1.3 and/or 3.1.2.1.1?] I did check the bug list,
but I didn't find anything relevant with a few cursory searches. Or is
it an implementation-dependent issue? If so, where does CLHS define the
relevant issues?

I thank you in advance for your consideration.

- Jack

From: Surendra Singhi
Subject: Re: Bug hunting/lexical confusion
Date: 
Message-ID: <r7a4x8ja.fsf@netscape.net>
"Jack" <············@msn.com> writes:

> A project of mine needs a zip file reader. I found the Common Lisp Zip
> Library (http://common-lisp.net/project/zip/ ) and decided to test it
> for my purposes. I ran preliminary tests (i.e., compiling and loading
> the library) in two different environments, Clisp 2.35 and SBCL 0.9.5
> (SBCL + XEmacs + Slime), with varying results. (Despite its power,
> (X)Emacs is SO F'IN CLUMSY--I CAN'T STAND IT! I'm STILL looking for a
> more comfortable editor [for Linux] with similar features.) An issue
> causing a failure on loading in Clisp raised some confusion about the
> standard vs. implementation. I haven't found relevant details in the
> Hyperspec, so I wondered if anyone might have the answer.

The clisp implementation of gray streams is different from that of other
lisps. Look at the implementation notes.

> First, since the zip library hasn't yet released support for Clisp and
> I hadn't yet seen the patch at the project's message archive, I hacked
> in some #clisp-dependent features, threw together something for the
> gray support in a file named clisp.lisp and called (asdf:operate
> 'load-op 'zip). It successfully compiled zip.lisp, but upon loading
> zip.fas, CLISP choked and kicked into the debugger giving the
> following:
>

I had implemented a patch to make it work in clisp, but for some reason
common-lisp was not archiving messages. Atleast two more people have sent
clisp related patches, but they are also not archived.


I am attaching the patches with this message.

cvs diff -- package.lisp (in directory C:\clisp-2.33.2\library\zip\)
Index: package.lisp
===================================================================
RCS file: /project/zip/cvsroot/zip/package.lisp,v
retrieving revision 1.2
diff -r1.2 package.lisp
29c29,30
<                 #-(or sbcl lispworks) ...
---
>       #+clisp :gray
>                 #-(or sbcl lispworks clisp) ...
31c32,33
<                 #:stream-write-sequence
---
>                 #-clisp #:stream-write-sequence
>       #+clisp #:stream-write-byte-sequence
34c36,37
<                 #:stream-read-sequence))
---
>       #+clisp #:stream-read-byte-sequence
>                 #-clisp #:stream-read-sequence))



cvs diff -- gray.lisp (in directory C:\clisp-2.33.2\library\zip\)
Index: gray.lisp
===================================================================
RCS file: /project/zip/cvsroot/zip/gray.lisp,v
retrieving revision 1.2
diff -r1.2 gray.lisp
6a7
> #-clisp
15a17,36
> #+clisp
> (defmethod stream-element-type
>     ((stream buffer-output-stream))
>   '(unsigned-byte 8))
>
> #+clisp
> (defmethod stream-write-byte-sequence
>     ((stream buffer-output-stream) seq &optional (start 0) end no-hang)
>   (replace (buf stream) seq
>      :start1 (pos stream)
>      :start2 start
>      :end2 end))
>
> #+clisp
> (defmethod stream-write-byte
>     ((stream buffer-output-stream) byte)
>   (prog1 (setf (aref (buf stream) (pos stream)) byte)
>     (incf (pos stream))))
>
>
30a52
> #-clisp
39a62,79
>            :start start
>            :end (+ start (min n max)))))
>     (incf (pos s) (- result start))
>     result))
>
> #+clisp
> (defmethod stream-element-type
>     ((s truncating-stream))
>   '(unsigned-byte 8))
>
> #+clisp
> (defmethod stream-read-byte-sequence
>     ((s truncating-stream) seq &optional (start 0) (end (length seq)) no-hang)
>   (let* ((n (- end start))
>    (max (- (size s) (pos s)))
>    (result
>     (read-sequence seq
>            (input-handle s)


The code for file clisp.lisp it is same as that of sbcl.lisp

(in-package :zip)


(defun default-external-format ()
:dummy)

(defun octets-to-string (octets ef)
(declare (ignore ef))
(let* ((m (length octets))
  (n (cond
       ((zerop m) 0)
       ((zerop (elt octets (1- m))) (1- m))
       (t m)))
  (result (make-string n)))
 (map-into result #'code-char octets)
 result))

(defun string-to-octets (string ef)
(declare (ignore ef))
(let ((result (make-array (1+ (length string))
             :element-type '(unsigned-byte 8)
             :initial-element 0)))
 (map-into result #'char-code string)
 result))


-- 
Surendra Singhi
http://www.public.asu.edu/~sksinghi/index.html

,----
| By all means marry; if you get a good wife, you'll be happy. If you
| get a bad one, you'll become a philosopher.  
|    -- Socrates
`----
From: Jack
Subject: Re: Bug hunting/lexical confusion
Date: 
Message-ID: <1130626537.836114.220440@g44g2000cwa.googlegroups.com>
Thanks for the patch, but I think you misunderstood my problem. I had
no problem adding feature macros where necessary, and I wrote my own
implementation for using gray stream functionality in clisp.lisp before
I saw a patch in the August archives of the project's message archives,
which I found through the project site I mentioned at the beginning of
my original post. In fact, my feature macros in package.lisp and my
home-brewed clisp.lisp very closely resemble those in your patch.

However, nothing in the patch addresses my question about the lexical
environment in the toplevel progn of the macro in question or whether
Clisp 2.35 might have a real bugger in it. Applying the patch wouldn't
address the load failure. Incidentally, I just submitted a bug report,
which addresses the issue and gives the developers enough to reproduce
the error, to the Clisp project at sourceforge.

I'll let them determine if they have a real issue, but I still wonder
about the correct behavior for the macro and my understanding of and
deductions about the correct behavior, re. the lexical environment of
and bindings in the top-level progn.

- Jack
From: Jack
Subject: Re: Bug hunting/lexical confusion
Date: 
Message-ID: <1130707468.916762.251050@o13g2000cwo.googlegroups.com>
My error--the bug watchers for Clisp at sourceforge marked it as a
duplicate of bug 836838. (I swear I searched for the word uninterned in
the bug tracker, but it came up empty. Of course, when I visually
scanned the open bugs, I didn't see it right there in front of me.)
Reading the other bug report helped clear up some confusion concerning
how it should work, and my analysis was on point.

- Jack