Re: [Chicken-hackers] [PATCH][5] Some FFI improvements

[LemonBoy]

On 05/29, Peter Bex wrote:
> On Sun, May 28, 2017 at 11:29:06PM +0200, lemonboy wrote:
> > Hello hackers,
> 
> Hi Lemonboy,
> 
> Thanks (again!) for your patches.  You're really putting in quite
> the effort.
> 
Thanks for the kind words :)
> I have two comments about the patch, and a more generic comment:
> 
> 1) I don't like the extra "mode" argument making its way into the
>     final-foreign-type API.  It has nothing to do with the _foreign_
>     type of the value, that's unchanged.  This is purely a scrutinizer
>     change, and it shouldn't "infect" code that deals only with foreign
>     types.
> 
Indeed, I wasn't too sure about where to place the check.
> 2) Your patch bails out if it encounters any conversion functions during
>     the lookup.  However, this is overly strict: the procedures
>     foreign-type-convert-{result,argument} will perform a "shallow"
>     lookup.  That means if you create a foreign type "alias" for another
>     type, the alias won't inherit the type converters of the other type.
>     Whether that's desirable is up for debate, of course, but that's how
>     it currently works.  So we can just use the "final" type if the
>     initial lookup returns no converters.
> 
Colour me surprised as I've never thought one could aliasing a foreign type,
I'll note this down.
> More generally, I think indiscriminantly adding (##core#the ...)
> annotations for foreign values is too aggressive.  It would be better
> to only do that when we can determine a proper result type.
> 
> Allowing an optional type annotation would allow the scrutinizer to
> analyse the converter's type.  Currently, returning '* just blocks the
> scrunitizer from performing its analysis.  In fact, it would be even
> better if we can use the foreign type's scrutiny type as information
> for the conversion procedures' input or output.  But that's just wishful
> thinking.
> 
This is an interesting idea, I think we can acheive this by teaching the
scrutinizer a new "type hole" annotation that's only handled when we deal with 
##core#the nodes. The gist of the idea is to stop discarding the type
approximation we've built for the expression inside the ##core#the node and to
"merge" it with the enforced one so that we can fill the holes in the latter
with the types from the former.
I hope I've conveyed the idea in a way that's somewhat understandable, I
realized a long ago I don't really have a way with words.
> For now, I think the attached patch will do, and has less impact because
> the change is more localised.  I struggled quite a bit to make this code
> readable, so if anyone knows how to improve this, please do!
> 
> Cheers,
> Peter