The system cffi-libffi allows calling functions with structures passed and returned by value. The cffi-libffi branch has been merged into master. Because bare structure reference is deprecated, the merge affects a lot code using CFFI, not just those calling functions with structures by value. Expect to see many style warnings in code that refers to bare structure names. Please test and report successes/problems. We hope to have a new release of CFFI soon.
See blog: http://lhealy.livejournal.com/2012/04/14/
Thank you. Liam
On Sat, 2012-04-14 at 15:13 -0400, Liam Healy wrote:
The system cffi-libffi allows calling functions with structures passed and returned by value. The cffi-libffi branch has been merged into master. Because bare structure reference is deprecated, the merge affects a lot code using CFFI, not just those calling functions with structures by value. Expect to see many style warnings in code that refers to bare structure names. Please test and report successes/problems. We hope to have a new release of CFFI soon.
Does that mean that CFFI now requires libffi to be present on a system and that it will be loaded into lisp process?
There is a separate system cffi-libffi which needs libffi. You may choose not to load it. However, there are changes to cffi itself as well, which is why testing even without using libffi is a good idea.
On Sat, Apr 14, 2012 at 3:24 PM, Kalyanov Dmitry kalyanov.dmitry@gmail.comwrote:
On Sat, 2012-04-14 at 15:13 -0400, Liam Healy wrote:
The system cffi-libffi allows calling functions with structures passed and returned by value. The cffi-libffi branch has been merged into master. Because bare structure reference is deprecated, the merge affects a lot code using CFFI, not just those calling functions with structures by value. Expect to see many style warnings in code that refers to bare structure names. Please test and report successes/problems. We hope to have a new release of CFFI soon.
Does that mean that CFFI now requires libffi to be present on a system and that it will be loaded into lisp process?
cffi-devel mailing list cffi-devel@common-lisp.net http://lists.common-lisp.net/cgi-bin/mailman/listinfo/cffi-devel
On Sat, Apr 14, 2012 at 8:24 PM, Kalyanov Dmitry kalyanov.dmitry@gmail.com wrote:
Does that mean that CFFI now requires libffi to be present on a system and that it will be loaded into lisp process?
No. There's a separate system named "cffi-libffi" that is optional.
returned by value. The cffi-libffi branch has been merged into master. Because bare structure reference is deprecated, the merge affects a lot code using CFFI, not just those calling functions with structures by value.
because so many people so often speak up _against_ change, and never for it, even if it's desirable and moves things forward...
let me voice my support here for making cffi better even by introducing some non 100% backwards compatible changes.
thanks!
On Apr 15, 2012 9:39 AM, "Attila Lendvai" attila.lendvai@gmail.com wrote:
let me voice my support here for making cffi better even by introducing some non 100% backwards compatible changes.
These changes *should* be backwards compatible though. :-)
let me voice my support here for making cffi better even by introducing some non 100% backwards compatible changes.
These changes *should* be backwards compatible though. :-)
oh well.
but still, you get the idea... :)
On Sun, Apr 15, 2012 at 1:31 PM, Attila Lendvai attila.lendvai@gmail.comwrote:
let me voice my support here for making cffi better even by introducing some non 100% backwards compatible changes.
These changes *should* be backwards compatible though. :-)
oh well.
but still, you get the idea... :)
-- attila
It may be surprising, but I've heard no objections yet. I do imagine people are going to gripe about the flood of style warnings (once this version gets into quicklisp). It will be interesting to see if code maintainers rewrite their code.
Liam
On 4/15/12 Apr 15 -3:38 AM, Attila Lendvai wrote:
returned by value. The cffi-libffi branch has been merged into master. Because bare structure reference is deprecated, the merge affects a lot code using CFFI, not just those calling functions with structures by value.
because so many people so often speak up _against_ change, and never for it, even if it's desirable and moves things forward...
let me voice my support here for making cffi better even by introducing some non 100% backwards compatible changes.
And let me voice *my* support here for people using CFFI in their ASDF systems also putting the :VERSION spec in :DEPENDS-ON!
That would be so nice.....
Cheers, r
On Sat, 2012-04-14 at 15:13 -0400, Liam Healy wrote:
The system cffi-libffi allows calling functions with structures passed and returned by value. The cffi-libffi branch has been merged into master. Because bare structure reference is deprecated, the merge affects a lot code using CFFI, not just those calling functions with structures by value. Expect to see many style warnings in code that refers to bare structure names. Please test and report successes/problems. We hope to have a new release of CFFI soon.
IMO the change to mem-aref is bad and will break a lot of code. Until now the contract of mem-aref was that it received a pointer to an array of the referenced type and returned a pointer offset into the array. The fact that it now returns a list is a gratuitous change, with no utility. If we actually want these semantics(not sure about it), it should be mem-aptr to implement them
IMO the change to mem-aref is bad and will break a lot of code. Until now the contract of mem-aref was that it received a pointer to an array of the referenced type and returned a pointer offset into the array. The fact that it now returns a list is a gratuitous change, with no utility. If we actually want these semantics(not sure about it), it should be mem-aptr to implement them
-- Stelian Ionescu a.k.a. fe[nl]ix Quidquid latine dictum sit, altum videtur. http://common-lisp.net/project/iolib
The meaning of mem-aref is that returns the object, not a pointer (unless the array is an array of pointers). It still does that. I'm not sure what you mean by "returns a list" unless it's the conversion of a foreign structure to a plist, which is the default representation of a foreign structure. You can define the appropriate translate method to return any representation of the structure you like, and mem-aref is still completely consistent with the previous meaning, that the object (not a pointer) is returned. We introduced mem-aptr to mean the value of the pointer (not the object) is wanted.
The only danger of breakage is that people previously used bare structure references to mean the pointer (because no other meaning was possible), and wanted the pointer back from mem-aref. That is still supported; if you use the bare structure and mem-aref, you'll get the pointer. If you use (:struct foo), you mean you want the structure, and you'll get it (as a plist or however you defined the translate method). If you want an array of pointers, you can specify (:pointer (:struct foo)) or just :pointer, if you want an array of structures and get a pointer back, you specify (:struct foo) and use mem-aptr.
The current behavior makes sense and is completely compatible with the old. There is no change to mem-aref.
Liam
On Thu, Apr 19, 2012 at 11:34 AM, Stelian Ionescu sionescu@cddr.org wrote:
IMO the change to mem-aref is bad and will break a lot of code. Until now the contract of mem-aref was that it received a pointer to an array of the referenced type and returned a pointer offset into the array. The fact that it now returns a list is a gratuitous change, with no utility. If we actually want these semantics(not sure about it), it should be mem-aptr to implement them
If it's not too difficult, can you extract a self-contained test case that has been broken?
Cheers,
On Thu, 2012-04-19 at 14:37 +0100, Luís Oliveira wrote:
On Thu, Apr 19, 2012 at 11:34 AM, Stelian Ionescu sionescu@cddr.org wrote:
IMO the change to mem-aref is bad and will break a lot of code. Until now the contract of mem-aref was that it received a pointer to an array of the referenced type and returned a pointer offset into the array. The fact that it now returns a list is a gratuitous change, with no utility. If we actually want these semantics(not sure about it), it should be mem-aptr to implement them
If it's not too difficult, can you extract a self-contained test case that has been broken?
(defcstruct timespec (sec :int64) (usec :int64))
(with-foreign-object (p '(:struct timespec) 2) (mem-aref p '(:struct timespec) 1))
In order not to break existing code, I think that mem-aref should continue to return a pointer in this case instead of a plist. That would allow cffi-using code to work as it is and not have to be rewritten(an example that was brought to my attention is https://gitorious.org/commonqt/commonqt/blobs/master/info.lisp#line312).
Since both functionalities will continue to be present in CFFI, it's better not to force users to review their code to decide, for each use of mem-aref, whether it needs to be converted to mem-aptr or not.
On Thu, Apr 19, 2012 at 2:50 PM, Stelian Ionescu sionescu@cddr.org wrote:
(with-foreign-object (p '(:struct timespec) 2) (mem-aref p '(:struct timespec) 1))
In order not to break existing code [...]
Existing code will not have this (:struct foo) syntax because it was introduced by the libffi merge. (mem-aref p 'timespec 1) should exhibit backwards-compatible behaviour.
Luís Oliveira <luismbo <at> gmail.com> writes:
On Thu, Apr 19, 2012 at 2:50 PM, Stelian Ionescu <sionescu <at> cddr.org>
wrote:
(with-foreign-object (p '(:struct timespec) 2) (mem-aref p '(:struct timespec) 1))
In order not to break existing code [...]
Existing code will not have this (:struct foo) syntax because it was introduced by the libffi merge. (mem-aref p 'timespec 1) should exhibit backwards-compatible behaviour.
Turns out, the problem is not with mem-aref, but with the mem-aref compile- macro. It binds *parse-bare-structs-as-pointers* to T, whereas mem-aref function doesn't, this affects the result of foreign-type-size.
(cffi:defcstruct foo (classid :short) (name :short) (methodid :short))(cffi:defcstruct foo (classid :short) (name :short)
(cffi:foreign-type-size 'foo) => 6
(let ((cffi::*parse-bare-structs-as-pointers* t)) (cffi:foreign-type-size 'foo)) => 8
On Thu, 2012-04-19 at 14:46 +0000, Stas Boukarev wrote:
Luís Oliveira <luismbo <at> gmail.com> writes:
On Thu, Apr 19, 2012 at 2:50 PM, Stelian Ionescu <sionescu <at> cddr.org>
wrote:
(with-foreign-object (p '(:struct timespec) 2) (mem-aref p '(:struct timespec) 1))
In order not to break existing code [...]
Existing code will not have this (:struct foo) syntax because it was introduced by the libffi merge. (mem-aref p 'timespec 1) should exhibit backwards-compatible behaviour.
Turns out, the problem is not with mem-aref, but with the mem-aref compile- macro. It binds *parse-bare-structs-as-pointers* to T, whereas mem-aref function doesn't, this affects the result of foreign-type-size.
Actually it's mem-aref that should bind *parse-bare-structs-as-pointers* to T, so I pushed the fix
Stelian Ionescu sionescu@cddr.org writes:
On Thu, 2012-04-19 at 14:46 +0000, Stas Boukarev wrote:
Luís Oliveira <luismbo <at> gmail.com> writes:
On Thu, Apr 19, 2012 at 2:50 PM, Stelian Ionescu <sionescu <at> cddr.org>
wrote:
(with-foreign-object (p '(:struct timespec) 2) (mem-aref p '(:struct timespec) 1))
In order not to break existing code [...]
Existing code will not have this (:struct foo) syntax because it was introduced by the libffi merge. (mem-aref p 'timespec 1) should exhibit backwards-compatible behaviour.
Turns out, the problem is not with mem-aref, but with the mem-aref compile- macro. It binds *parse-bare-structs-as-pointers* to T, whereas mem-aref function doesn't, this affects the result of foreign-type-size.
Actually it's mem-aref that should bind *parse-bare-structs-as-pointers* to T, so I pushed the fix
This breaks it.
(cffi:defcstruct foo (a :short) (b :short))
;; After binding *parse-bare-structs-as-pointers* to T in the mem-aref function:
(cffi:with-foreign-object (var 'foo 20) (let ((type 'foo)) (cffi:mem-aref var type))) ;; new cffi => #.(SB-SYS:INT-SAP #X00000000) ;; old cffi and new with a constant type return an expected address
;; Previous problems I originally described:
(cffi:with-foreign-object (var 'foo 2) (- (cffi-sys:pointer-address (cffi:mem-aref var 'foo 1)) (cffi-sys:pointer-address (cffi:mem-aref var 'foo 0))))
;; old cffi => 4 ;; new cffi => 8
;; With mem-aptr works as expected:
(cffi:with-foreign-object (var 'foo 2) (- (cffi-sys:pointer-address (cffi:mem-aptr var 'foo 1)) (cffi-sys:pointer-address (cffi:mem-aptr var 'foo 0))))
;; new cffi => 4
Stas Boukarev stassats@gmail.com writes:
Stelian Ionescu sionescu@cddr.org writes:
On Thu, 2012-04-19 at 14:46 +0000, Stas Boukarev wrote:
Luís Oliveira <luismbo <at> gmail.com> writes:
On Thu, Apr 19, 2012 at 2:50 PM, Stelian Ionescu <sionescu <at> cddr.org>
wrote:
(with-foreign-object (p '(:struct timespec) 2) (mem-aref p '(:struct timespec) 1))
In order not to break existing code [...]
Existing code will not have this (:struct foo) syntax because it was introduced by the libffi merge. (mem-aref p 'timespec 1) should exhibit backwards-compatible behaviour.
Turns out, the problem is not with mem-aref, but with the mem-aref compile- macro. It binds *parse-bare-structs-as-pointers* to T, whereas mem-aref function doesn't, this affects the result of foreign-type-size.
Actually it's mem-aref that should bind *parse-bare-structs-as-pointers* to T, so I pushed the fix
This breaks it.
And the same problem with *parse-bare-structs-as-pointers* being different is also present in the setf macro for mem-aref.
On Tue, 2012-05-01 at 00:00 +0400, Stas Boukarev wrote:
Stas Boukarev stassats@gmail.com writes:
Stelian Ionescu sionescu@cddr.org writes:
On Thu, 2012-04-19 at 14:46 +0000, Stas Boukarev wrote:
Luís Oliveira <luismbo <at> gmail.com> writes:
On Thu, Apr 19, 2012 at 2:50 PM, Stelian Ionescu <sionescu <at> cddr.org>
wrote:
(with-foreign-object (p '(:struct timespec) 2) (mem-aref p '(:struct timespec) 1))
In order not to break existing code [...]
Existing code will not have this (:struct foo) syntax because it was introduced by the libffi merge. (mem-aref p 'timespec 1) should exhibit backwards-compatible behaviour.
Turns out, the problem is not with mem-aref, but with the mem-aref compile- macro. It binds *parse-bare-structs-as-pointers* to T, whereas mem-aref function doesn't, this affects the result of foreign-type-size.
Actually it's mem-aref that should bind *parse-bare-structs-as-pointers* to T, so I pushed the fix
This breaks it.
And the same problem with *parse-bare-structs-as-pointers* being different is also present in the setf macro for mem-aref.
Ok, I reverted that patch because it's now obvious that I misunderstood the problem, the bug seems to be in the compiler-macro:
CFFI> (cffi:defcstruct foo (a :uint8)) => (:STRUCT FOO) CFFI> (cffi:with-foreign-object (var 'foo 2) (declare (inline mem-aref)) (- (cffi-sys:pointer-address (cffi:mem-aref var 'foo 1)) (cffi-sys:pointer-address (cffi:mem-aref var 'foo 0)))) => 8 CFFI> (cffi:with-foreign-object (var 'foo 2) (declare (notinline mem-aref)) (- (cffi-sys:pointer-address (cffi:mem-aref var 'foo 1)) (cffi-sys:pointer-address (cffi:mem-aref var 'foo 0)))) => 1
This fixes it for me
(defmethod foreign-type-size ((type symbol)) (let ((*parse-bare-structs-as-pointers* nil)) (foreign-type-size (parse-type type))))
The key code path is is this in define-compiler-macro mem-aref
(mem-ref ,ptr ,type (* ,index ,(foreign-type-size (eval type))))
When TYPE is 'FOO, the (foreign-type-size T) method is used, which uses PARSE-TYPE, which returns (:pointer (:struct foo)), which in in case of 'foo is incorrect as 'FOO is a structure of size 6, and not a pointer.
At Mon, 30 Apr 2012 23:29:13 +0200, Stelian Ionescu wrote:
On Tue, 2012-05-01 at 00:00 +0400, Stas Boukarev wrote:
Stas Boukarev stassats@gmail.com writes:
Stelian Ionescu sionescu@cddr.org writes:
On Thu, 2012-04-19 at 14:46 +0000, Stas Boukarev wrote:
Luís Oliveira <luismbo <at> gmail.com> writes:
On Thu, Apr 19, 2012 at 2:50 PM, Stelian Ionescu <sionescu <at> cddr.org>
wrote:
> (with-foreign-object (p '(:struct timespec) 2) > (mem-aref p '(:struct timespec) 1)) > > In order not to break existing code [...]
Existing code will not have this (:struct foo) syntax because it was introduced by the libffi merge. (mem-aref p 'timespec 1) should exhibit backwards-compatible behaviour.
Turns out, the problem is not with mem-aref, but with the mem-aref compile- macro. It binds *parse-bare-structs-as-pointers* to T, whereas mem-aref function doesn't, this affects the result of foreign-type-size.
Actually it's mem-aref that should bind *parse-bare-structs-as-pointers* to T, so I pushed the fix
This breaks it.
And the same problem with *parse-bare-structs-as-pointers* being different is also present in the setf macro for mem-aref.
Ok, I reverted that patch because it's now obvious that I misunderstood the problem, the bug seems to be in the compiler-macro:
CFFI> (cffi:defcstruct foo (a :uint8)) => (:STRUCT FOO) CFFI> (cffi:with-foreign-object (var 'foo 2) (declare (inline mem-aref)) (- (cffi-sys:pointer-address (cffi:mem-aref var 'foo 1)) (cffi-sys:pointer-address (cffi:mem-aref var 'foo 0)))) => 8 CFFI> (cffi:with-foreign-object (var 'foo 2) (declare (notinline mem-aref)) (- (cffi-sys:pointer-address (cffi:mem-aref var 'foo 1)) (cffi-sys:pointer-address (cffi:mem-aref var 'foo 0)))) => 1
-- Stelian Ionescu a.k.a. fe[nl]ix Quidquid latine dictum sit, altum videtur. http://common-lisp.net/project/iolib
On Mon, Apr 30, 2012 at 11:02 PM, Max Mikhanosha max@openchat.com wrote:
This fixes it for me
(defmethod foreign-type-size ((type symbol)) (let ((*parse-bare-structs-as-pointers* nil)) (foreign-type-size (parse-type type))))
Thanks everyone for digging into this. While this fix does indeed fix the problem (nice catch!), using this special variable *PARSE-BARE-STRUCTS-AS-POINTERS* clearly turned out to be an overcomplicated and brittle approach.
I've pushed a rewrite that should be much more robust.
The key code path is is this in define-compiler-macro mem-aref
(mem-ref ,ptr ,type (* ,index ,(foreign-type-size (eval type))))
This highlights an important issue. We were not testing the non-compiler-macro code paths on SBCL. I've submitted a patch for RT that should help with that. As Stelian mentioned on IRC, we should perhaps control the deactivation of compiler macros explicitly rather than rely on EVAL not to use them. Curiously, forcing SBCL to use its interpreter revealed a possible bug: https://bugs.launchpad.net/sbcl/+bug/992362.
Luís Oliveira luismbo@gmail.com writes:
On Mon, Apr 30, 2012 at 11:02 PM, Max Mikhanosha max@openchat.com wrote:
This fixes it for me
(defmethod foreign-type-size ((type symbol)) (let ((*parse-bare-structs-as-pointers* nil)) (foreign-type-size (parse-type type))))
Thanks everyone for digging into this. While this fix does indeed fix the problem (nice catch!), using this special variable *PARSE-BARE-STRUCTS-AS-POINTERS* clearly turned out to be an overcomplicated and brittle approach.
I've pushed a rewrite that should be much more robust.
Commonqt seems to work fine now. When will there be a new release with this fix? So that it can be appear in the next quicklisp update.
On Tue, May 1, 2012 at 7:20 AM, Stas Boukarev stassats@gmail.com wrote:
Commonqt seems to work fine now. When will there be a new release with this fix? So that it can be appear in the next quicklisp update.
The latest release does not contain any of this new support for structures as values, so it shouldn't be an issue.
On Tue, May 1, 2012 at 7:13 AM, Luís Oliveira luismbo@gmail.com wrote:
On Tue, May 1, 2012 at 7:20 AM, Stas Boukarev stassats@gmail.com wrote:
Commonqt seems to work fine now. When will there be a new release with this fix? So that it can be appear in the next quicklisp update.
The latest release does not contain any of this new support for structures as values, so it shouldn't be an issue.
I assume this question was about the fix to master, i.e., including cffi-libffi. I hope that pretty soon (with errors confirmed fixed) this will be tagged as a new release (1.0.0) and QL will pick it up on the next cycle.
Liam
On Tue, May 1, 2012 at 9:29 PM, Liam Healy lnp@healy.washington.dc.us wrote:
I assume this question was about the fix to master, i.e., including cffi-libffi. I hope that pretty soon (with errors confirmed fixed) this will be tagged as a new release (1.0.0) and QL will pick it up on the next cycle.
Let's target the June dist. (Perhaps July if too many bugs crop up.)
On Thu, Apr 19, 2012 at 9:50 AM, Stelian Ionescu sionescu@cddr.org wrote:
On Thu, 2012-04-19 at 14:37 +0100, Luís Oliveira wrote:
On Thu, Apr 19, 2012 at 11:34 AM, Stelian Ionescu sionescu@cddr.org
wrote:
IMO the change to mem-aref is bad and will break a lot of code. Until now the contract of mem-aref was that it received a pointer to an array of the referenced type and returned a pointer offset into the array. The fact that it now returns a list is a gratuitous change, with no utility. If we actually want these semantics(not sure about it), it should be mem-aptr to implement them
If it's not too difficult, can you extract a self-contained test case that has been broken?
(defcstruct timespec (sec :int64) (usec :int64))
(with-foreign-object (p '(:struct timespec) 2) (mem-aref p '(:struct timespec) 1))
In order not to break existing code, I think that mem-aref should continue to return a pointer in this case instead of a plist. That would allow cffi-using code to work as it is and not have to be rewritten(an example that was brought to my attention is https://gitorious.org/commonqt/commonqt/blobs/master/info.lisp#line312).
Since both functionalities will continue to be present in CFFI, it's better not to force users to review their code to decide, for each use of mem-aref, whether it needs to be converted to mem-aptr or not.
-- Stelian Ionescu a.k.a. fe[nl]ix Quidquid latine dictum sit, altum videtur. http://common-lisp.net/project/iolib
As Luis pointed out, no code should be broken because '(:struct foo) wasn't used before. The example you point to has a symbol representing the type '|struct MethodMap| which I guess is being interpreted as if it were'(:struct MethodMap). I'm not sure, but that's the only way I can see this not being interpreted as a bare structure. If it were interpreted as a bare structure, everything should work as before.
Liam
-
Attila Lendvai -
Kalyanov Dmitry -
Liam Healy -
Liam Healy -
Luís Oliveira -
Max Mikhanosha -
Robert Goldman -
Stas Boukarev -
Stelian Ionescu