The setf-expansion for nested FOREIGN-SLOT-VALUE calls ist not quite correct:
CFFI> (macroexpand '(setf (foreign-slot-value (foreign-slot-value ptr `struct1 'slot1) `struct2 'slot2) val)) (LET* () (MULTIPLE-VALUE-BIND (#:G1997) VAL (PROGN (FOREIGN-SLOT-SET #:G1997 (FOREIGN-SLOT-VALUE PTR) 'STRUCT2 'SLOT2) #:G1997))) T ^^^^^^^^^^^^^^^^^^^^^^^^
Defining the expander like
(define-setf-expander foreign-slot-value (ptr type slot-name &environment env) (multiple-value-bind (temps vals stores store-form access-form) (get-setf-expansion ptr env) (declare (ignore store-form stores)) (let ((type-tmp (gensym "TYPE-")) (slot-name-tmp (gensym "SLOT-NAME-")) (store (gensym "STORE-"))) (values `(,type-tmp ,slot-name-tmp ,@temps) `(,type ,slot-name ,@vals) `(,store) `(progn (foreign-slot-set ,store ,access-form ,type-tmp ,slot-name-tmp) ,store) `(foreign-slot-value ,access-form ,type-tmp ,slot-name-tmp)))))
should do the job a bit better, though it might be optimized in some way (this is my first try to define a setf expander).
Btw. the manual contains an example use of FOREIGN-SLOT-VALUE with the simplified syntax
(foreign-slot-value ptr 'struct 'slot-1 'slot-2)
I think CFFI has all the informations at hand to handle this, so will it be implemented one day?
Regards,
Marco Gidde
At least two more expanders should also be corrected:
(define-setf-expander mem-ref (ptr type &optional (offset 0) &environment env) "SETF expander for MEM-REF that doesn't rebind TYPE. This is necessary for the compiler macro on MEM-SET to be able to open-code (SETF MEM-REF) forms." (multiple-value-bind (temps vals stores store-form access-form) (get-setf-expansion ptr env) (declare (ignore store-form stores)) (let ((type-tmp (gensym "TYPE-")) (offset-tmp (gensym "OFFSET-")) (store (gensym "STORE-"))) (values `(,type-tmp ,offset-tmp ,@temps) `(,type ,offset ,@vals) `(,store) `(progn (mem-set ,store ,access-form ,type-tmp ,offset-tmp) ,store) `(mem-ref ,access-form ,type-tmp ,offset-tmp)))))
(define-setf-expander mem-aref (ptr type &optional (index 0) &environment env) "SETF expander for MEM-AREF." (multiple-value-bind (temps vals stores store-form access-form) (get-setf-expansion ptr env) (declare (ignore store-form stores)) (let ((type-tmp (gensym "TYPE-")) (index-tmp (gensym "INDEX-")) (store (gensym "STORE-"))) (values `(,type-tmp ,index-tmp ,@temps) `(,type ,index ,@vals) `(,store) `(progn ,(if (constantp type) (if (constantp index) `(mem-set ,store ,access-form ,type-tmp ,(* (eval index) (foreign-type-size (eval type)))) `(mem-set ,store ,access-form ,type-tmp (* ,index-tmp ,(foreign-type-size (eval type))))) `(mem-set ,store ,access-form ,type-tmp (* ,index-tmp (foreign-type-size ,type-tmp)))) ,store) `(mem-aref ,access-form ,type-tmp ,index-tmp)))))
I guess the implementation dependend setf-expanders for %mem-ref should also be adapted. It's quite easy: instead of the first example in the CLHS take second ;-)
Regards,
Marco Gidde
Marco Gidde marco.gidde@tiscali.de writes:
At least two more expanders should also be corrected:
(define-setf-expander mem-ref (ptr type &optional (offset 0) &environment env) "SETF expander for MEM-REF that doesn't rebind TYPE. This is necessary for the compiler macro on MEM-SET to be able to open-code (SETF MEM-REF) forms."
Yes, thanks for the bug report! Things get a bit messier than your fix though because, like that docstring above says, we don't want to rebind the type (when it's constant) otherwise the compiler macro on MEM-SET won't kick in. The same applies for all the other setf expanders.
I guess the implementation dependend setf-expanders for %mem-ref should also be adapted. It's quite easy: instead of the first example in the CLHS take second ;-)
Yeah, that second example was most helpful. :-)
Anyway, this should be fixed now and I added a couple of regression tests too. Thanks again. Changelog follows (with an unrelated tweak to load-foreign-library to accept pathnames, as pointed out by Kenny).
Sun Sep 25 20:36:02 WEST 2005 Luis Oliveira loliveira@common-lisp.net * Fixed bogus getters in setf expanders.
Bug report and initial bugfixes courtesy of Marco Gidde.
- fixed setf expanders for mem-ref, mem-aref, and foreign-slot-value. (also mem-aref was evaluating the type argument twice) - likewise fixed cmucl's, openmcl's and sbcl's setf expanders for %mem-ref. - regression tests: mem-aref.eval-type-x2, mem-ref.nested, mem-aref.nested and struct.nested-setf.
Sun Sep 25 05:42:42 WEST 2005 Luis Oliveira loliveira@common-lisp.net * load-foreign-library
- extend load-foreign-library to accept a pathname as an argument. - change tests/bindings.lisp to pass a pathname.
Luis Oliveira luismbo@gmail.com writes:
Things get a bit messier than your fix though because, like that docstring above says, we don't want to rebind the type (when it's constant) otherwise the compiler macro on MEM-SET won't kick in. The same applies for all the other setf expanders.
I was sure I missed something, but I just wanted to compile a simple example instead of diving too deep into CFFI and do some premature optimizations :-)
Thanks for the fix!
Marco Gidde marco.gidde@tiscali.de writes:
The setf-expansion for nested FOREIGN-SLOT-VALUE calls ist not quite correct:
[...]
should do the job a bit better, though it might be optimized in some way (this is my first try to define a setf expander).
Thanks, I'll have a look at this tomorrow. I need to read define-setf-expander's docs, heh. /My/ first try to define a setf expander was mem-aref's and that was just by looking at mem-ref's expander.
Btw. the manual contains an example use of FOREIGN-SLOT-VALUE with the simplified syntax
(foreign-slot-value ptr 'struct 'slot-1 'slot-2)
I think CFFI has all the informations at hand to handle this, so will it be implemented one day?
Yes, one day. :-)
-
Luis Oliveira -
Marco Gidde