From: William H. N. <wil...@ai...> - 2002-10-23 13:06:15
|
Christophe pointed out that the DEFSTRUCT example I posted to the cmucl-imp mailing list fails in SBCL.:-( A simpler example of the failure is (DEFUN FOO () (DEFSTRUCT FOO X)). The problem is that the build-a-constructor code is confidently trying to find the layout at compile time because it implicitly assumes that the EVAL-WHEN (COMPILE) stuff setting up the layout got executed already. And of course when the DEFSTRUCT macro expands in a non-top-level context this is not correct. I've made a brute-force patch for this, below, which I'm inclined to add before 0.7.9, maybe tomorrow. If anyone wants to check to see whether I've made any new conceptual errors, it'd probably be a good idea. If performance matters in this case, there is probably some cute way to use *FORWARD-REFERENCED-LAYOUTS* tricks to make it better, but I thought the mindlessly straightforward code below was more appropriate for a last-minute patch. Index: src/code/defstruct.lisp =================================================================== RCS file: /cvsroot/sbcl/sbcl/src/code/defstruct.lisp,v retrieving revision 1.46 diff -u -r1.46 defstruct.lisp --- src/code/defstruct.lisp 17 Oct 2002 19:56:05 -0000 1.46 +++ src/code/defstruct.lisp 23 Oct 2002 12:56:54 -0000 @@ -27,10 +27,36 @@ (t res)))) ;;; Delay looking for compiler-layout until the constructor is being -;;; compiled, since it doesn't exist until after the EVAL-WHEN (COMPILE) -;;; stuff is compiled. +;;; compiled, since it doesn't exist until after the EVAL-WHEN +;;; (COMPILE) stuff is compiled. (Or, in the oddball case when +;;; DEFSTRUCT is executing in a non-toplevel context, the +;;; compiler-layout still doesn't exist at compilation time, and we +;;; delay still further.) (sb!xc:defmacro %delayed-get-compiler-layout (name) - `',(compiler-layout-or-lose name)) + (let ((layout (info :type :compiler-layout name))) + (cond (layout + ;; ordinary case: When the DEFSTRUCT is at top level, + ;; then EVAL-WHEN (COMPILE) stuff will have set up the + ;; layout for us to use. + (unless (typep (layout-info layout) 'defstruct-description) + (error "Class is not a structure class: ~S" name)) + `,layout) + (t + ;; KLUDGE: In the case that DEFSTRUCT is not at top-level + ;; the layout doesn't exist at compile time. In that case + ;; we laboriously look it up at run time. This code will + ;; run on every constructor call and will likely be quite + ;; slow, so if anyone cares about performance of + ;; non-toplevel DEFSTRUCTs, it should be rewritten to be + ;; cleverer. -- WHN 2002-10-23 + (sb!c::compiler-note + "implementation limitation: ~ + Non-toplevel DEFSTRUCT constructors are slow.") + (let ((layout (gensym "LAYOUT"))) + `(let ((,layout (info :type :compiler-layout ',name))) + (unless (typep (layout-info ,layout) 'defstruct-description) + (error "Class is not a structure class: ~S" ',name)) + ,layout)))))) ;;; Get layout right away. (sb!xc:defmacro compile-time-find-layout (name) -- William Harold Newman <wil...@ai...> rather lucky so far PGP key fingerprint 85 CE 1C BA 79 8D 51 8C B9 25 FB EE E0 C3 E5 7C |