On Jul 30, 2014, at 17:14 , Peter Karman <[email protected]> wrote:
> [email protected] wrote on 7/30/14 9:43 AM:
>> Adjust for Clownfish fixes to overridden aliases
>>
>> The callback from C to Perl calls the alias '_make_compiler' now, so
>> this is the method that has to be overridden.
>>
>> This is probably confusing for authors of custom queries in Perl and a
>> better fix would be nice.
>>
>
> As one of those authors, I agree. A better fix would be nice.
>
> What was the original impetus for making the Perl method "private" (leading
> underscore) if it is part of the public API?
Make_Compiler is declared in Clownfish as follows:
public abstract incremented Compiler*
Make_Compiler(Query *self, Searcher *searcher, float boost,
bool subordinate = false);
For the Perl bindings, we want to use a default value if ‘boost’ is undefined
or missing (the Query’s boost value). C doesn’t have the notion of undefined or
missing arguments. So ‘Make_Compiler’ is aliased to ‘_make_compiler’ and a Perl
wrapper named ‘make_compiler’ is used to set the default.
Until now, Clownfish didn’t account for host method aliases when calling
overridden Perl methods, so ‘make_compiler’ was called. This happens to work as
expected in this case, but generally it is wrong. Clownfish should invoke
methods overridden from Perl under their aliased name since it can’t assume
that there’s a wrapper with the original name doing the right thing. I fixed
this in the Clownfish branch ‘overridden_aliases’.
I can see two solutions.
A. Keep the Perl wrapper possibly using different names for the original method
and the wrapper. Unfortunately, this wouldn’t be backward compatible. At least,
we could choose whether to break either implementors or users of
‘make_compiler’.
B. Emulate the default value on the C level, for example by using a special
float constant to signal that the default should be used:
public abstract incremented Compiler*
Make_Compiler(Query *self, Searcher *searcher,
float boost = special_value,
bool subordinate = false);
Then we could drop the alias and wrapper without affecting backward
compatibility. Using zero or a negative number as default value would be easy
but might be confusing. A value like NaN (not a number) would be the best
choice in my opinion but this requires more work on the Clownfish side.
Nick
