From c1cfe1b9f634bdaa1f607795f46dafa52d6620da Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Tue, 10 Dec 2013 16:13:25 -0800 Subject: [PATCH] Handle inherited fields correctly. --- src/wren_common.h | 5 ++ src/wren_compiler.c | 86 +++++++++++++++++++ src/wren_compiler.h | 15 ++++ src/wren_value.c | 9 +- src/wren_value.h | 7 ++ src/wren_vm.c | 5 +- test/inheritance/inherit_fields.wren | 35 ++++++++ .../inherit_methods.wren} | 0 test/inheritance/is.wren | 16 ++++ test/is.wren | 18 ---- 10 files changed, 174 insertions(+), 22 deletions(-) create mode 100644 test/inheritance/inherit_fields.wren rename test/{class/inheritance.wren => inheritance/inherit_methods.wren} (100%) create mode 100644 test/inheritance/is.wren diff --git a/src/wren_common.h b/src/wren_common.h index 4b26a222..c4816fac 100644 --- a/src/wren_common.h +++ b/src/wren_common.h @@ -1,6 +1,8 @@ #ifndef wren_common_h #define wren_common_h +// TODO(bob): Use stdbool.h and `bool` for bools instead of int/1/0. + // This header contains macros and defines used across the entire Wren // implementation. In particular, it contains "configuration" defines that // control how Wren works. Some of these are only used while hacking on @@ -8,6 +10,9 @@ // // This header is *not* intended to be included by code outside of Wren itself. +// TODO(bob): Prefix these with Wren and use #if instead of #ifdef. That way +// the flags can be set externally. + // Define this to stress test the GC. It will perform a collection before every // allocation. //#define DEBUG_GC_STRESS diff --git a/src/wren_compiler.c b/src/wren_compiler.c index bae2a6fa..cbb9ca63 100644 --- a/src/wren_compiler.c +++ b/src/wren_compiler.c @@ -1821,3 +1821,89 @@ ObjFn* wrenCompile(WrenVM* vm, const char* source) return parser.hasError ? NULL : compiler.fn; } + +void wrenBindMethod(ObjClass* classObj, ObjFn* fn) +{ + int ip = 0; + for (;;) + { + Code instruction = fn->bytecode[ip++]; + switch (instruction) + { + // Instructions with no arguments: + case CODE_NULL: + case CODE_FALSE: + case CODE_TRUE: + case CODE_DUP: + case CODE_POP: + case CODE_IS: + case CODE_CLOSE_UPVALUE: + case CODE_RETURN: + break; + + // Instructions with one argument: + case CODE_CONSTANT: + case CODE_CLASS: + case CODE_SUBCLASS: + case CODE_LIST: + case CODE_LOAD_LOCAL: + case CODE_STORE_LOCAL: + case CODE_LOAD_UPVALUE: + case CODE_STORE_UPVALUE: + case CODE_LOAD_GLOBAL: + case CODE_STORE_GLOBAL: + case CODE_CALL_0: + case CODE_CALL_1: + case CODE_CALL_2: + case CODE_CALL_3: + case CODE_CALL_4: + case CODE_CALL_5: + case CODE_CALL_6: + case CODE_CALL_7: + case CODE_CALL_8: + case CODE_CALL_9: + case CODE_CALL_10: + case CODE_CALL_11: + case CODE_CALL_12: + case CODE_CALL_13: + case CODE_CALL_14: + case CODE_CALL_15: + case CODE_CALL_16: + case CODE_JUMP: + case CODE_LOOP: + case CODE_JUMP_IF: + case CODE_AND: + case CODE_OR: + ip++; + break; + + // Instructions with two arguments: + case CODE_METHOD_INSTANCE: + case CODE_METHOD_STATIC: + case CODE_METHOD_CTOR: + ip += 2; + break; + + case CODE_CLOSURE: + { + int constant = fn->bytecode[ip++]; + ObjFn* loadedFn = AS_FN(fn->constants[constant]); + ip += loadedFn->numUpvalues; + break; + } + + case CODE_LOAD_FIELD: + case CODE_STORE_FIELD: + // Shift this class's fields down past the inherited ones. + fn->bytecode[ip++] += classObj->superclass->numFields; + break; + + case CODE_END: + return; + + default: + ASSERT(0, "Unknown instruction."); + break; + } + } +} diff --git a/src/wren_compiler.h b/src/wren_compiler.h index 3e6b5848..3ec6f75b 100644 --- a/src/wren_compiler.h +++ b/src/wren_compiler.h @@ -23,4 +23,19 @@ // execute that code when invoked. ObjFn* wrenCompile(WrenVM* vm, const char* source); +// When a class is defined, its superclass is not known until runtime since +// class definitions are just imperative statements. Most of the bytecode for a +// a method doesn't care, but there are two places where it matters: +// +// - To load or store a field, we need to know its index of the field in the +// instance's field array. We need to adjust this so that subclass fields +// are positioned after superclass fields, and we don't know this until the +// superclass is known. +// +// - Superclass calls need to know which superclass to dispatch to. +// +// We could handle this dynamically, but that adds overhead. Instead, when a +// method is bound, we walk the bytecode for the function and patch it up. +void wrenBindMethod(ObjClass* classObj, ObjFn* fn); + #endif diff --git a/src/wren_value.c b/src/wren_value.c index 660c5da6..54455e49 100644 --- a/src/wren_value.c +++ b/src/wren_value.c @@ -24,11 +24,12 @@ static ObjClass* newClass(WrenVM* vm, ObjClass* metaclass, initObj(vm, &obj->obj, OBJ_CLASS); obj->metaclass = metaclass; obj->superclass = superclass; - obj->numFields = numFields; - // Inherit methods from its superclass (unless it's Object, which has none). if (superclass != NULL) { + obj->numFields = superclass->numFields + numFields; + + // Inherit methods from its superclass. for (int i = 0; i < MAX_SYMBOLS; i++) { obj->methods[i] = superclass->methods[i]; @@ -36,6 +37,9 @@ static ObjClass* newClass(WrenVM* vm, ObjClass* metaclass, } else { + obj->numFields = numFields; + + // Object has no superclass, so just clear these out. for (int i = 0; i < MAX_SYMBOLS; i++) { obj->methods[i].type = METHOD_NONE; @@ -57,7 +61,6 @@ ObjClass* wrenNewClass(WrenVM* vm, ObjClass* superclass, int numFields) pinObj(vm, (Obj*)metaclass, &pinned); ObjClass* classObj = newClass(vm, metaclass, superclass, numFields); - classObj->numFields = numFields; unpinObj(vm); diff --git a/src/wren_value.h b/src/wren_value.h index 6a924c63..e403247c 100644 --- a/src/wren_value.h +++ b/src/wren_value.h @@ -177,6 +177,9 @@ typedef enum typedef struct { MethodType type; + + // The method function itself. The [type] determines which field of the union + // is used. union { Primitive primitive; @@ -192,7 +195,11 @@ typedef struct sObjClass Obj obj; struct sObjClass* metaclass; struct sObjClass* superclass; + + // The number of fields needed for an instance of this class, including all + // of its superclass fields. int numFields; + // TODO(bob): Hack. Probably don't want to use this much space. Method methods[MAX_SYMBOLS]; } ObjClass; diff --git a/src/wren_vm.c b/src/wren_vm.c index 64a9c048..70156c35 100644 --- a/src/wren_vm.c +++ b/src/wren_vm.c @@ -710,6 +710,9 @@ Value interpret(WrenVM* vm, Value function) break; } + ObjFn* methodFn = IS_FN(method) ? AS_FN(method) : AS_CLOSURE(method)->fn; + wrenBindMethod(classObj, methodFn); + classObj->methods[symbol].fn = method; DISPATCH(); } @@ -1057,11 +1060,11 @@ Value interpret(WrenVM* vm, Value function) void wrenCallFunction(Fiber* fiber, Value function, int numArgs) { + // TODO(bob): Check for stack overflow. fiber->frames[fiber->numFrames].fn = function; fiber->frames[fiber->numFrames].ip = 0; fiber->frames[fiber->numFrames].stackStart = fiber->stackSize - numArgs; - // TODO(bob): Check for stack overflow. fiber->numFrames++; } diff --git a/test/inheritance/inherit_fields.wren b/test/inheritance/inherit_fields.wren new file mode 100644 index 00000000..212e4898 --- /dev/null +++ b/test/inheritance/inherit_fields.wren @@ -0,0 +1,35 @@ +class Foo { + foo(a, b) { + _field1 = a + _field2 = b + } + + fooPrint { + io.write(_field1) + io.write(_field2) + } +} + +class Bar is Foo { + bar(a, b) { + _field1 = a + _field2 = b + } + + barPrint { + io.write(_field1) + io.write(_field2) + } +} + +var bar = Bar.new +bar.foo("foo 1", "foo 2") +bar.bar("bar 1", "bar 2") + +bar.fooPrint +// expect: foo 1 +// expect: foo 2 + +bar.barPrint +// expect: bar 1 +// expect: bar 2 diff --git a/test/class/inheritance.wren b/test/inheritance/inherit_methods.wren similarity index 100% rename from test/class/inheritance.wren rename to test/inheritance/inherit_methods.wren diff --git a/test/inheritance/is.wren b/test/inheritance/is.wren new file mode 100644 index 00000000..d08f62cd --- /dev/null +++ b/test/inheritance/is.wren @@ -0,0 +1,16 @@ +class A {} +class B is A {} +class C is B {} +var a = A.new +var b = B.new +var c = C.new + +io.write(a is A) // expect: true +io.write(a is B) // expect: false +io.write(a is C) // expect: false +io.write(b is A) // expect: true +io.write(b is B) // expect: true +io.write(b is C) // expect: false +io.write(c is A) // expect: true +io.write(c is B) // expect: true +io.write(c is C) // expect: true diff --git a/test/is.wren b/test/is.wren index 6fd0afeb..4b720b25 100644 --- a/test/is.wren +++ b/test/is.wren @@ -20,24 +20,6 @@ io.write((fn 1) is Object) // expect: true io.write("s" is Object) // expect: true io.write(123 is Object) // expect: true -// Inheritance. -class A {} -class B is A {} -class C is B {} -var a = A.new -var b = B.new -var c = C.new - -io.write(a is A) // expect: true -io.write(a is B) // expect: false -io.write(a is C) // expect: false -io.write(b is A) // expect: true -io.write(b is B) // expect: true -io.write(b is C) // expect: false -io.write(c is A) // expect: true -io.write(c is B) // expect: true -io.write(c is C) // expect: true - // TODO(bob): Non-class on RHS. // TODO(bob): Precedence and associativity. // TODO(bob): Metaclasses ("Num is Class").