[lucy-dev] Re: Adjust for Clownfish fixes to overridden aliases

[Nick Wellnhofer <we...@aevum.de>]

On 31/07/2014 04:00, Marvin Humphrey wrote:
> It is still important to come up with a general solution to the problem
> of autogenerated bindings which do not behave quite right.  Our current
> hack -- using CFC to autogenerate a "private" helper (i.e. prepended
> with an underscore) and then creating a wrapper with the "real" name --
> has been revealed as unworkable.  However, that hack is still used in
> many places -- see much of Lucy.pm and Clownfish.pm.

Clownfish.pm seems to be OK. There are no such aliases, only a couple of 
helper functions that are called from C.

> The underlying issue is that now that Nick has fixed the callback
> behavior to invoke the alias (THANK YOU!) `bind_method` should not be
> used to create helpers -- only "real" methods.  I suggest a workaround
> of creating those private helpers with hand-rolled XS -- which, unlike
> abusing `bind_method` for that purpose, will not have the side effect of
> changing what Perl-space name gets invoked for callbacks.

Regarding the hand-rolled XS, there's another subtle issue that I encountered 
when working on callbacks for excluded methods. For hand-rolled XS methods, we 
call '$binding->exclude_method' to tell CFC not to autogenerate an XSUB for 
us. But that's not enough to determine whether callbacks for overriding from 
Perl should be allowed. There are cases where we want callbacks to work, like 
Query#Make_Compiler. But in other cases, the hand-rolled XS has incompatible 
parameters so overriding would break. So the API should look like:

     # Don't generate XSUB and callback. A method with the same name
     # can be defined via custom XS or Perl but callbacks won't work.
     $binding->exclude_method(...);

     # Don't generate XSUB but create callback.
     $binding->exclude_xsub(...);

There's also a third case where we want to autogenerate an XSUB but no 
callback. I can't think of an immediate use case, but at some point, we might 
want to allow custom callbacks.

Nick

Re: [lucy-dev] Re: Adjust for Clownfish fixes to overridden aliases

[Marvin Humphrey <ma...@rectangular.com>]

On Wed, Aug 20, 2014 at 2:51 AM, Nick Wellnhofer <we...@aevum.de> wrote:
> On 31/07/2014 15:25, Nick Wellnhofer wrote:
>>
>> On 31/07/2014 04:00, Marvin Humphrey wrote:
>>>
>>> It is still important to come up with a general solution to the problem
>>> of autogenerated bindings which do not behave quite right.  Our current
>>> hack -- using CFC to autogenerate a "private" helper (i.e. prepended
>>> with an underscore) and then creating a wrapper with the "real" name --
>>> has been revealed as unworkable.  However, that hack is still used in
>>> many places -- see much of Lucy.pm and Clownfish.pm.
>>
>> Clownfish.pm seems to be OK. There are no such aliases, only a couple of
>> helper functions that are called from C.
>
> Having a closer look, there are two cases where Clownfish uses Perl
> wrappers: String#Clone and VArray#Clone. But the XSUBs aren't excluded so we
> have both an XSub and a Perl sub. This looks like a bug.

It's an old hack:

        # Defeat obscure bugs in the XS auto-generation by redefining clone().
        # (Because of how the typemap works for String*,
        # the auto-generated methods return UTF-8 Perl scalars rather than
        # actual String objects.)
        no warnings 'redefine';
        sub clone { shift->_clone(@_) }

Without that hack, calling `$clownfish_string->clone` returns a Perl scalar
rather than another Clownfish::String, because cfish_String* is mapped to a
UTF-8 Perl scalar in our clownfish-to-perl data conversion routines.

The reason that we have to redefine the Perl-space method clone() for
Clownfish::String is because Clownfish's XSUB binding generator doesn't offer
per-class granularity -- it's either generate clone() XSUBs for *all*
subclasses of Obj, or none.

Marvin Humphrey

Re: [lucy-dev] Re: Adjust for Clownfish fixes to overridden aliases

[Nick Wellnhofer <we...@aevum.de>]

On 31/07/2014 15:25, Nick Wellnhofer wrote:
> On 31/07/2014 04:00, Marvin Humphrey wrote:
>> It is still important to come up with a general solution to the problem
>> of autogenerated bindings which do not behave quite right.  Our current
>> hack -- using CFC to autogenerate a "private" helper (i.e. prepended
>> with an underscore) and then creating a wrapper with the "real" name --
>> has been revealed as unworkable.  However, that hack is still used in
>> many places -- see much of Lucy.pm and Clownfish.pm.
>
> Clownfish.pm seems to be OK. There are no such aliases, only a couple of
> helper functions that are called from C.

Having a closer look, there are two cases where Clownfish uses Perl wrappers: 
String#Clone and VArray#Clone. But the XSUBs aren't excluded so we have both 
an XSub and a Perl sub. This looks like a bug.

> Regarding the hand-rolled XS, there's another subtle issue that I encountered
> when working on callbacks for excluded methods. For hand-rolled XS methods, we
> call '$binding->exclude_method' to tell CFC not to autogenerate an XSUB for
> us. But that's not enough to determine whether callbacks for overriding from
> Perl should be allowed. There are cases where we want callbacks to work, like
> Query#Make_Compiler. But in other cases, the hand-rolled XS has incompatible
> parameters so overriding would break. So the API should look like:
>
>      # Don't generate XSUB and callback. A method with the same name
>      # can be defined via custom XS or Perl but callbacks won't work.
>      $binding->exclude_method(...);
>
>      # Don't generate XSUB but create callback.
>      $binding->exclude_xsub(...);
>
> There's also a third case where we want to autogenerate an XSUB but no
> callback. I can't think of an immediate use case, but at some point, we might
> want to allow custom callbacks.

I created a branch overridden_exclusions laying some groundwork and 
consolidating some code. The only effective change for now is that the XSUBs 
for To_Host are removed, so I think it's safe to merge.

Nick