Hi Vegard, (I'm cc'ing Eduard on the args map question.)
On Sat, 2009-08-22 at 23:39 +0200, Vegard Nossum wrote: > @@ -371,7 +371,56 @@ int vm_class_link(struct vm_class *vmc, const > struct cafebabe_class *class) > } > } > > - vmc->methods = malloc(sizeof(*vmc->methods) * class->methods_count); > + unsigned int nr_miranda_methods = 0; > + struct vm_method **miranda_methods = NULL; > + > + /* If in any of the superinterfaces we find a method which is not > + * defined in this class file, we need to add a "miranda" method. > + * Note that we don't need to do this recursively for all super- > + * interfaces because they will have already done this very same > + * procedure themselves. */ > + for (unsigned int i = 0; i < class->interfaces_count; ++i) { > + struct vm_class *vmi = vmc->interfaces[i]; > + > + for (unsigned int j = 0; j < vmi->nr_methods; ++j) { > + struct vm_method *vmm = &vmi->methods[j]; > + > + assert(vmm->name); > + assert(vmm->type); The assertions look useless. Why do we have them here? > + > + /* We need this "manual" recursive lookup because we > + * haven't initialized this class' list of methods > + * yet... */ > + unsigned int index = 0; > + if (!cafebabe_class_get_method(vmc->class, > + vmm->name, vmm->type, &index)) > + { Too deep nesting here? The brace is on the wrong line and you might want to fix the cafebabe API _not_ to return index in arguments and probably. > + continue; > + } > + > + if (!vmc->super) > + continue; > + > + if (vm_class_get_method_recursive(vmc->super, > + vmm->name, vmm->type)) > + { Opening brace on the wrong line. > + continue; > + } > + > + /* XXX: Use generic resizable array? */ Hey, either fix it or drop the annoying XXX comment. There's obviously nothing wrong with using realloc() here except that it's buggy and doesn't check for NULL. > + miranda_methods = realloc(miranda_methods, > + sizeof(*miranda_methods) > + * (nr_miranda_methods + 1)); > + > + miranda_methods[nr_miranda_methods] = vmm; > + > + ++nr_miranda_methods; > + } > + } > + > + vmc->nr_methods = class->methods_count + nr_miranda_methods; > + > + vmc->methods = malloc(sizeof(*vmc->methods) * vmc->nr_methods); > if (!vmc->methods) { > NOT_IMPLEMENTED; > return -1; > @@ -387,7 +436,28 @@ int vm_class_link(struct vm_class *vmc, const struct > cafebabe_class *class) > > vmm->itable_index = itable_hash(vmm); > > - if (vm_method_prepare_jit(&vmc->methods[i])) { > + /* XXX: Only if the method actually has code. */ Hmm? What does this mean? > + if (vm_method_prepare_jit(vmm)) { > + NOT_IMPLEMENTED; NOT_IMPLEMENTED is banned from new code. > + return -1; > + } > + } > + > + for (uint16_t i = 0; i < nr_miranda_methods; ++i) { > + struct vm_method *vmm = &vmc->methods[class->methods_count + i]; > + > + if (vm_method_init_from_interface(vmm, vmc, > + class->methods_count + i, miranda_methods[i])) > + { Opening brace on the wrong line. > + NOT_IMPLEMENTED; Banned from new code. > + return -1; > + } > + > + /* Share with above? */ > + vmm->itable_index = itable_hash(vmm); > + > + /* XXX: We _know_ that this method doesn't have any code. */ This is the third or fourth XXX added by this patch. Needless to say, it's annoying. > + if (vm_method_prepare_jit(vmm)) { > NOT_IMPLEMENTED; NOT_IMPLEMENTED. > return -1; > } > @@ -829,9 +899,13 @@ struct vm_method *vm_class_get_method(const struct > vm_class *vmc, > if (vmc->kind != VM_CLASS_KIND_REGULAR) > return NULL; > > - unsigned int index = 0; > - if (!cafebabe_class_get_method(vmc->class, name, type, &index)) > - return &vmc->methods[index]; > + /* Could do binary search here if we wanted. */ You forgot XXX here. Seriously though, I'm not sure I see the value of this comment. If it's a _known_ problem, it should be fixed and it's not as if perf won't be able to find this call-site if it turns out to be a problem later on. > + for (unsigned int i = 0; i < vmc->nr_methods; ++i) { > + struct vm_method *vmm = &vmc->methods[i]; > + > + if (!strcmp(vmm->name, name) && !strcmp(vmm->type, type)) > + return vmm; > + } > > return NULL; > } > diff --git a/vm/method.c b/vm/method.c > index 095c1de..4be9c31 100644 > --- a/vm/method.c > +++ b/vm/method.c > @@ -141,6 +141,37 @@ int vm_method_init(struct vm_method *vmm, > return 0; > } > > +int vm_method_init_from_interface(struct vm_method *vmm, struct vm_class > *vmc, > + unsigned int method_index, struct vm_method *interface_method) The indentation of the method arguments is really strange because there's not much visual clue to separate the second line from actual code. > +{ > + /* NOTE: If we ever keep reference counts on loaded classes, we should > + * perhaps _copy_ the interformation from the interface method instead > + * of just grabbing a reference to the same information. */ > + > + vmm->class = vmc; > + vmm->method_index = method_index; > + vmm->method = interface_method->method; > + > + vmm->name = interface_method->name; > + vmm->type = interface_method->type; > + > + vmm->args_count = interface_method->args_count; > + vmm->is_vm_native = false; > + > + /* XXX: Do we need to duplicate args_map? */ AFAICT, yes. Eduard? > + /* All interface methods are abstract. */ > + { What's with the braces? > + vmm->code_attribute.max_stack = 0; > + vmm->code_attribute.max_locals = vmm->args_count; > + > + vmm->line_number_table_attribute.line_number_table_length = 0; > + vmm->line_number_table_attribute.line_number_table = NULL; > + } > + > + return 0; > +} > + > int vm_method_prepare_jit(struct vm_method *vmm) > { > struct compilation_unit *cu; ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ Jatovm-devel mailing list Jatovm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jatovm-devel