[cffi-devel] Re: CFFI::CANONICALIZE with arguments (NIL)

[Lou Vanek]

Luís Oliveira wrote:

> On 2006-maj-20, at 15:37, Lou Vanek wrote:
>
>> +++ cffi/src/early-types.lisp   2006-05-20 08:47:59.708847000 -0400
>> @@ -266,7 +266,10 @@
>>
>>  (defmethod canonicalize ((type foreign-type-alias))
>>    "Return the built-in type keyword for TYPE."
>> -  (canonicalize (actual-type type)))
>> +  (let ((atype (actual-type type)))
>> +         (if (null atype)
>> +               (format *standard-error* "WARNING! null base-type.")
>> +               (canonicalize atype))))
>
>
>
> I'll probably put an (assert (not (null (actual-type type)))) there instead.


I don't think you want an assert here.

There is one instance when you actually want this to be null (temporarily).
The uffi-array-type class initially does not specify an actual-type
until it's "initialize-instance :after" method, so I think canonicalize needs
to allow null actual-types, at least until after all of the constructors
have finished running. (Canonicalize is called prior to "initialize-instance
:after" is invoked.)

As a possible work-around, you may want to give uffi-array-type a temporary
actual-type until the real actual-type is set.


>
>> diff -urN --exclude '*.lib' --exclude '*.fas' --exclude files
>> cffi.clean/src/utils.lisp cffi/src/utils.lisp
>> --- cffi.clean/src/utils.lisp   2006-05-20 08:35:15.130722000 -0400
>> +++ cffi/src/utils.lisp 2006-05-20 09:06:14.990097000 -0400
>> @@ -39,7 +39,8 @@
>>             #:let-when
>>             #:bif
>>             #:post-incf
>> -           #:single-bit-p))
>> +           #:single-bit-p
>> +                  #:getenv))
>
>
>
> Hmmm why are you moving GETENV into the cffi-utils package? (Hmm, maybe this
package should be renamed to cffi-internal-utils).


After reviewing my clsql code I think I changed the getenv call in clsql.asd.
Yeah, you're right, this getenv move isn't necessary for stock clsql code.

>
>> +++ cffi/uffi-compat/uffi-compat.lisp  2006-05-20  09:06:27.255722000 -0400
>> @@ -90,7 +90,6 @@
>>     #:foreign-library-types
>>
>>     ;; os
>> -   #:getenv
>>     #:run-shell-command
>>     ))
>
>
>
> And why are you removing it from uffi-compat? It's part of UFFI 1.5.7  (at 
least).


See above.

>
>> @@ -135,7 +134,7 @@
>>    (:documentation "UFFI's :array type."))
>>
>>  (defmethod initialize-instance :after ((self uffi-array-type) &key)
>> -  (setf (cffi::actual-type self) (cffi::find-type :pointer)))
>> +  (setf (cffi::actual-type self) (cffi::parse-type :pointer)))
>
>
>
> Ah ha! Now there's the bug. Well spotted. Thanks. :-)


Your welcome.

>
>> @@ -143,7 +142,7 @@
>>  (defmethod cffi::aggregatep ((type uffi-array-type))
>>    t)
>>
>> -(cffi::define-type-spec-parser uffi-array (element-type count)
>> +(cffi::define-type-spec-parser uffi-array (element-type &optional  count)
>>    (make-instance 'uffi-array-type :element-type element-type
>>                   :nelems (or count 1)))
>
>
>
> What's the rationale behind this change? I'm guessing it has something to do
with def-array-pointer, but I think there's more to it.
>

This change was necessary for those times when you don't know how large
the array is going to be. Specifically, for this line in mysql-api.lisp:

(uffi:def-array-pointer mysql-row (* :unsigned-char))

Otherwise, since "count" is unknown, lisp will stop with an error because
the lamba list takes two arguments (element-type and count) but only
element-type is known.


>> --- cffi.clean/uffi.asd 1969-12-31 19:00:00.000000000 -0500
>> +++ cffi/uffi.asd       2006-05-20 09:15:36.990097000 -0400
>> @@ -0,0 +1,7 @@
>> +;;;-*- Mode: Lisp; Package: COMMON-LISP-USER -*-
>> +
>> +(defpackage :asdf-uffi (:use #:asdf #:cl))
>> +(in-package :asdf-uffi)
>> +(defsystem uffi :depends-on (:cffi-uffi-compat))
>> +
>> +;;; End file uffi.asd
>
>
>
> A couple of people have suggested this. I don't think cffi's root  directory
is a good place for this. Any suggestions? Perhaps inside uffi-compat or a new
contrib directoy?


It doesn't make any difference to me as long as it isn't buried.
I like the idea of putting in in uffi-compat since I would notice
it there because that's one of the first places I would look for it.
But I would mention this in the README, as well.