>> On 30/03/2012 23:31, Marvin Humphrey wrote:
>
>>> This loop construct would make more sense.
>>>
>>> FOR class IN classes
>>> FOR method IN class.methods
>>> IF class.method_needs_binding(method)
>>> generate_binding(method)
A patch implementing this loop logic is below my sig. However, it causes an
interesting set of test failures. Here's one of the failing tests:
t/523-and_query.t ..................... 1/11
Abstract method 'Make_Matcher' not defined by Lucy::Search::ANDCompiler
lucy_Compiler_make_matcher at autogen/source/parcel.c line 24614
at t/523-and_query.t line 65
# Looks like you planned 11 tests but ran 8.
# Looks like your test exited with 255 just after 8.
The reason this is happening is we are invoking ANDCompiler#Make_Matcher()
from Perl-space, but instead of getting ANDCompiler's implementation, we're
getting the parent class's abstract implementation (which throws an "abstract
method" error).
Like roughly half of the 300 or so classes in the Lucy core, ANDCompiler does
not have a specific Perl binding. To clear up this specific test failure, we
can add one:
--- a/perl/buildlib/Lucy/Build/Binding/Search.pm
+++ b/perl/buildlib/Lucy/Build/Binding/Search.pm
@@ -23,6 +23,7 @@ sub bind_all {
my $class = shift;
$class->bind_andmatcher;
$class->bind_andquery;
+ $class->bind_andcompiler;
$class->bind_bitvecmatcher;
$class->bind_collector;
$class->bind_bitcollector;
@@ -96,6 +97,14 @@ END_CONSTRUCTOR
Clownfish::CFC::Binding::Perl::Class->register($binding);
}
+sub bind_andcompiler {
+ my $binding = Clownfish::CFC::Binding::Perl::Class->new(
+ parcel => "Lucy",
+ class_name => "Lucy::Search::ANDCompiler",
+ );
+ Clownfish::CFC::Binding::Perl::Class->register($binding);
+}
+
sub bind_bitvecmatcher {
my $binding = Clownfish::CFC::Binding::Perl::Class->new(
parcel => "Lucy",
However, that only treats the symptom, not the disease.
Under the previous loop logic, *all* implementations of Make_Matcher() would
get method bindings:
FOR class IN classes
FOR method IN class.methods
IF method.novel OR (class.parent && class.parent.included)
FOR descendant IN class.descendants <------ includes ANDCompiler
generate_binding(method)
Under the new loop logic, bindings are only generated for manually registered
classes:
FOR class IN classes <------ doesn't include ANDCompiler
FOR method IN class.methods
IF class.method_needs_binding(method)
generate_binding(method)
This problem is not unique to ANDCompiler, but happens for any class which has
not had bindings specifically requested.
I suspect that the remedy is to automatically generate bindings for *all*
classes and methods in the hierarchy.
Marvin Humphrey
----------------------------------------------------------------------------
--- a/clownfish/src/CFCPerlClass.c
+++ b/clownfish/src/CFCPerlClass.c
@@ -255,7 +255,6 @@ CFCPerlClass_method_bindings(CFCPerlClass *self) {
const char *class_name = self->class_name;
size_t num_bound = 0;
CFCMethod **fresh_methods = CFCClass_fresh_methods(client);
- CFCClass **descendants = CFCClass_tree_to_ladder(client);
CFCPerlMethod **bound
= (CFCPerlMethod**)CALLOCATE(1, sizeof(CFCPerlMethod*));
@@ -267,17 +266,33 @@ CFCPerlClass_method_bindings(CFCPerlClass *self) {
const char *alias = CFCMethod_micro_sym(method);
const char *meth_name = CFCMethod_get_macro_sym(method);
- // Only deal with methods when they are novel (i.e. first declared)
- // or the parent class is included.
- if (!CFCMethod_novel(method) && !parent_included) { continue; }
+ // Skip all methods for included classes.
+ if (CFCClass_included(self->client)) { continue; }
// Skip private methods.
if (CFCSymbol_private((CFCSymbol*)method)) { continue; }
+ // Find the class where the method was first declared.
+ CFCClass *originator = client;
+ CFCMethod *novel_method = method;
+ while (1) {
+ CFCClass *parent = CFCClass_get_parent(originator);
+ if (!parent) { break; }
+ CFCMethod *maybe_novel = CFCClass_method(parent, meth_name);
+ if (!maybe_novel) { break; }
+ originator = parent;
+ novel_method = maybe_novel;
+ }
+ CFCPerlClass *originator_binding
+ = CFCPerlClass_singleton(CFCClass_get_class_name(originator));
+
+ // If the class where the method was declared was not bound,
skip.
+ if (!originator_binding) { continue; }
+
// Skip methods which have been explicitly excluded.
int is_excluded = 0;
- for (size_t j = 0; j < self->num_excluded; j++) {
- if (strcmp(self->excluded[j], meth_name) == 0) {
+ for (size_t j = 0; j < originator_binding->num_excluded; j++) {
+ if (strcmp(originator_binding->excluded[j], meth_name) == 0) {
is_excluded = 1;
break;
}
@@ -292,11 +307,11 @@ CFCPerlClass_method_bindings(CFCPerlClass *self) {
}
// See if the user wants the method to have a specific alias.
- for (size_t j = 0; j < self->num_methods; j++) {
- const char *maybe = self->meth_names[j];
+ for (size_t j = 0; j < originator_binding->num_methods; j++) {
+ const char *maybe = originator_binding->meth_names[j];
if (strcmp(maybe, meth_name) == 0) {
- if (self->meth_aliases[j]) {
- alias = self->meth_aliases[j];
+ if (originator_binding->meth_aliases[j]) {
+ alias = originator_binding->meth_aliases[j];
}
}
}
@@ -306,26 +321,15 @@ CFCPerlClass_method_bindings(CFCPerlClass *self) {
* the object using VTable method dispatch. Doing things this way
* allows SUPER:: invocations from Perl-space to work properly.
*/
- for (size_t j = 0; descendants[j] != NULL; j++) {
- CFCClass *descendant = descendants[j];
- CFCMethod *real_method
- = CFCClass_fresh_method(descendant, meth_name);
- if (!real_method) { continue; }
-
- // Create the binding, add it to the array.
- CFCPerlMethod *meth_binding =
CFCPerlMethod_new(real_method, alias);
- size_t size = (num_bound + 2) * sizeof(CFCPerlMethod*);
- bound = (CFCPerlMethod**)REALLOCATE(bound, size);
- bound[num_bound] = meth_binding;
- num_bound++;
- bound[num_bound] = NULL;
- }
-
+ CFCPerlMethod *meth_binding = CFCPerlMethod_new(method, alias);
+ size_t size = (num_bound + 2) * sizeof(CFCPerlMethod*);
+ bound = (CFCPerlMethod**)REALLOCATE(bound, size);
+ bound[num_bound] = meth_binding;
+ num_bound++;
+ bound[num_bound] = NULL;
}
FREEMEM(fresh_methods);
- FREEMEM(descendants);
-
return bound;
}
