Re: [PATCH] vm: add unimplemented interface methods to abstract classes
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))
+
[PATCH] vm: add unimplemented interface methods to abstract classes
We use the term miranda method here as a method which is omitted by
the compiler (because the method is declared in an interface and the
implementing class is abstract), and which needs to be inserted by the
VM.
Reported-by: Tomek Grabiec tgrab...@gmail.com
Signed-off-by: Vegard Nossum vegard.nos...@gmail.com
---
include/vm/class.h |1 +
include/vm/method.h |3 ++
vm/class.c | 88 ++
vm/method.c | 31 ++
4 files changed, 116 insertions(+), 7 deletions(-)
diff --git a/include/vm/class.h b/include/vm/class.h
index 46b34b2..4451680 100644
--- a/include/vm/class.h
+++ b/include/vm/class.h
@@ -52,6 +52,7 @@ struct vm_class {
unsigned int nr_interfaces;
struct vm_class **interfaces;
struct vm_field *fields;
+ unsigned int nr_methods;
struct vm_method *methods;
unsigned int object_size;
diff --git a/include/vm/method.h b/include/vm/method.h
index 90a4fef..54c2b94 100644
--- a/include/vm/method.h
+++ b/include/vm/method.h
@@ -53,6 +53,9 @@ struct vm_method {
int vm_method_init(struct vm_method *vmm,
struct vm_class *vmc, unsigned int method_index);
+int vm_method_init_from_interface(struct vm_method *vmm, struct vm_class *vmc,
+ unsigned int method_index, struct vm_method *interface_method);
+
static inline bool vm_method_is_public(struct vm_method *vmm)
{
return vmm-method-access_flags CAFEBABE_METHOD_ACC_PUBLIC;
diff --git a/vm/class.c b/vm/class.c
index 4c77dd3..87868e4 100644
--- a/vm/class.c
+++ b/vm/class.c
@@ -74,7 +74,7 @@ setup_vtable(struct vm_class *vmc)
}
vtable_size = 0;
- for (uint16_t i = 0; i vmc-class-methods_count; ++i) {
+ for (uint16_t i = 0; i vmc-nr_methods; ++i) {
struct vm_method *vmm = vmc-methods[i];
if (super) {
@@ -101,7 +101,7 @@ setup_vtable(struct vm_class *vmc)
super_vtable-native_ptr[i]);
/* Our methods */
- for (uint16_t i = 0; i vmc-class-methods_count; ++i) {
+ for (uint16_t i = 0; i vmc-nr_methods; ++i) {
struct vm_method *vmm = vmc-methods[i];
vtable_setup_method(vmc-vtable,
@@ -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);
+
+ /* 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))
+ {
+ continue;
+ }
+
+ if (!vmc-super)
+ continue;
+
+ if (vm_class_get_method_recursive(vmc-super,
+ vmm-name, vmm-type))
+ {
+ continue;
+ }
+
+ /* XXX: Use generic resizable array? */
+ 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. */
+ if (vm_method_prepare_jit(vmm)) {
+ NOT_IMPLEMENTED;
+
