[cffi-devel] MEM-REF and BARE-STRUCT-TYPE-P issue
Hi all. #1 In src/types.lisp there is following code in function MEM-REF: .... (if (aggregatep parsed-type) (if (bare-struct-type-p parsed-type) .... and its compiler macro has similiar code. However, the gf bare-struct-type-p has no method for foreign-array-type whose objects will obviously pass the aggregatep test. So it breaks some code (on ccl). I suggest that case be added as a method too?(What should it (and even more user-defined types) return?) Or should the checking code be modified here? (As i understood, the whole bare struct mechanism is for compatibility...) #2 And another thing, i've realized that in my last modification to the STRUCTURE-BY-VALUE-P function (the modification is already pushed to the repo) i should have tested to see if it is a bare struct type. Without the check it will put some deprecated style code into using libffi, which also works correctly but involves unnecessary libffi dependency. Should I fix that? Regards, Charles Lew -- Wir müssen wissen; wir werden wissen! CrLF.0710
On Sat, Jan 19, 2013 at 1:25 AM, CRLF0710 <crlf0710@gmail.com> wrote:
Hi all.
#1 In src/types.lisp there is following code in function MEM-REF: .... (if (aggregatep parsed-type) (if (bare-struct-type-p parsed-type) .... and its compiler macro has similiar code. However, the gf bare-struct-type-p has no method for foreign-array-type whose objects will obviously pass the aggregatep test. So it breaks some code (on ccl). I suggest that case be added as a method too?(What should it (and even more user-defined types) return?) Or should the checking code be modified here? (As i understood, the whole bare struct mechanism is for compatibility...)
I would prefer to leave the question of aggregate definition to someone else, as I am not clear on what constitutes an aggregate. However, if you find code breaking, please post a "before and after" minimal example with the previous correct result and the current wrong result. If you have a proposed method to fix the problem, can you add that to your branch and do a pull request?
#2 And another thing, i've realized that in my last modification to the STRUCTURE-BY-VALUE-P function (the modification is already pushed to the repo) i should have tested to see if it is a bare struct type. Without the check it will put some deprecated style code into using libffi, which also works correctly but involves unnecessary libffi dependency. Should I fix that?
Yes, please.
Regards, Charles Lew
Liam
2013/1/19 Liam Healy <lnp@healy.washington.dc.us>
I would prefer to leave the question of aggregate definition to someone else, as I am not clear on what constitutes an aggregate. However, if you find code breaking, please post a "before and after" minimal example with the previous correct result and the current wrong result. If you have a proposed method to fix the problem, can you add that to your branch and do a pull request?
Ah, my English is not very good. Actually i meant the code i was writing in some personal projects are broken and i think it was cffi's bug, sorry. I can't do a pull request until i knew what is the correct thing to do (and wrote it down). Yes, there are several methods to fix it, but that depends on what we think BARE-STRUCT-TYPE-P should do. Solution #1 (defmethod bare-struct-type-p ((x foreign-type)) <return some proper value here>) Now we provide a default value. Solution #2 (defmethod bare-struct-type-p ((x foreign-array-type)) <return some proper value here>) Add we are asking user to implement bare-struct-type-p for their type before they can safely call MEM-REF Solution #3 Modify the MEM-REF code to see if it is a foreign-struct-type or typedef into a foreign-struct-type first AND call bare-struct-type-p I'm not sure which is better. Actually i'm not sure what it means to ask a non-struct type if it is bare(maybe the <proper value> is nil). But i think solution #1 is the simplest?
Yes, please.
OK. The fix should use BARE-STRUCT-TYPE-P, so after the fix above this will be fixed in a minute. -- Wir müssen wissen; wir werden wissen! CrLF.0710
On Sun, Jan 20, 2013 at 3:24 AM, CRLF0710 <crlf0710@gmail.com> wrote:
I'm not sure which is better. Actually i'm not sure what it means to ask a non-struct type if it is bare(maybe the <proper value> is nil). But i think solution #1 is the simplest?
It seems more convenient to have bare-struct-type-p return nil for non-struct types. -- Luís Oliveira http://r42.eu/~luis/
Good, i'll write that. By the way, i saw the launchpad bug tracker, so i've opened a issue there. https://bugs.launchpad.net/cffi/+bug/1102247 2013/1/21 Luís Oliveira <luismbo@gmail.com>
-- Wir müssen wissen; wir werden wissen! CrLF.0710
On Sat, Jan 19, 2013 at 6:25 AM, CRLF0710 <crlf0710@gmail.com> wrote:
#1 In src/types.lisp there is following code in function MEM-REF: .... (if (aggregatep parsed-type) (if (bare-struct-type-p parsed-type) .... and its compiler macro has similiar code. However, the gf bare-struct-type-p has no method for foreign-array-type whose objects will obviously pass the aggregatep test. So it breaks some code (on ccl).
That's definitely sounds like a bug. Can you open an issue on github? Cheers, -- Luís Oliveira http://r42.eu/~luis/
On Sun, Jan 20, 2013 at 3:00 AM, CRLF0710 <crlf0710@gmail.com> wrote:
I'm glad to, but hey, cffi on github didn't enable the Issues page, so i can't...
Oops, I meant launchpad. :-) -- Luís Oliveira http://r42.eu/~luis/
participants (3)
-
CRLF0710 -
Liam Healy -
Luís Oliveira