From 1a27593993abf186bd5f997dea7bcc9089a837bb Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Sat, 12 Sep 2015 10:14:04 -0700 Subject: [PATCH] Error if a class tries to inherit from null. Fix #246. --- src/vm/wren_compiler.c | 41 +++++++++---------- src/vm/wren_opcodes.h | 9 ++-- src/vm/wren_vm.c | 20 ++++----- .../inheritance/inherit_from_null.wren | 1 + 4 files changed, 32 insertions(+), 39 deletions(-) create mode 100644 test/language/inheritance/inherit_from_null.wren diff --git a/src/vm/wren_compiler.c b/src/vm/wren_compiler.c index 5e4523d6..cea15297 100644 --- a/src/vm/wren_compiler.c +++ b/src/vm/wren_compiler.c @@ -1020,9 +1020,9 @@ static int emitJump(Compiler* compiler, Code instruction) // Creates a new constant for the current value and emits the bytecode to load // it from the constant table. -static void emitConstant(Compiler* compiler, Value value) +static void emitConstant(Compiler* compiler, Value value) { - int constant = addConstant(compiler, value); + int constant = addConstant(compiler, value); // Compile the code to load the constant. emitShortArg(compiler, CODE_CONSTANT, constant); @@ -1790,6 +1790,15 @@ static void loadThis(Compiler* compiler) } } +// Pushes the value for a module-level variable implicitly imported from core. +static void loadCoreVariable(Compiler* compiler, const char* name) +{ + int symbol = wrenSymbolTableFind(&compiler->parser->module->variableNames, + name, strlen(name)); + ASSERT(symbol != -1, "Should have already defined core name."); + emitShortArg(compiler, CODE_LOAD_MODULE_VAR, symbol); +} + // A parenthesized expression. static void grouping(Compiler* compiler, bool allowAssignment) { @@ -1800,13 +1809,8 @@ static void grouping(Compiler* compiler, bool allowAssignment) // A list literal. static void list(Compiler* compiler, bool allowAssignment) { - // Load the List class. - int listClassSymbol = wrenSymbolTableFind( - &compiler->parser->module->variableNames, "List", 4); - ASSERT(listClassSymbol != -1, "Should have already defined 'List' variable."); - emitShortArg(compiler, CODE_LOAD_MODULE_VAR, listClassSymbol); - // Instantiate a new list. + loadCoreVariable(compiler, "List"); callMethod(compiler, 0, "new()", 5); // Compile the list elements. Each one compiles to a ".add()" call. @@ -1836,13 +1840,8 @@ static void list(Compiler* compiler, bool allowAssignment) // A map literal. static void map(Compiler* compiler, bool allowAssignment) { - // Load the Map class. - int mapClassSymbol = wrenSymbolTableFind( - &compiler->parser->module->variableNames, "Map", 3); - ASSERT(mapClassSymbol != -1, "Should have already defined 'Map' variable."); - emitShortArg(compiler, CODE_LOAD_MODULE_VAR, mapClassSymbol); - // Instantiate a new map. + loadCoreVariable(compiler, "Map"); callMethod(compiler, 0, "new()", 5); // Compile the map elements. Each one is compiled to just invoke the @@ -2125,7 +2124,7 @@ static void null(Compiler* compiler, bool allowAssignment) // A number or string literal. static void literal(Compiler* compiler, bool allowAssignment) { - emitConstant(compiler, compiler->parser->value); + emitConstant(compiler, compiler->parser->value); } static void super_(Compiler* compiler, bool allowAssignment) @@ -3002,8 +3001,8 @@ static bool method(Compiler* compiler, ClassCompiler* classCompiler, if (isForeign) { // Define a constant for the signature. - emitConstant(compiler, wrenNewString(compiler->parser->vm, - fullSignature, length)); + emitConstant(compiler, wrenNewString(compiler->parser->vm, + fullSignature, length)); // We don't need the function we started compiling in the parameter list // any more. @@ -3042,8 +3041,8 @@ static void classDefinition(Compiler* compiler, bool isForeign) int slot = declareNamedVariable(compiler); // Make a string constant for the name. - emitConstant(compiler, wrenNewString(compiler->parser->vm, - compiler->parser->previous.start, compiler->parser->previous.length)); + emitConstant(compiler, wrenNewString(compiler->parser->vm, + compiler->parser->previous.start, compiler->parser->previous.length)); // Load the superclass (if there is one). if (match(compiler, TOKEN_IS)) @@ -3052,8 +3051,8 @@ static void classDefinition(Compiler* compiler, bool isForeign) } else { - // Create the empty class. - emit(compiler, CODE_NULL); + // Implicitly inherit from Object. + loadCoreVariable(compiler, "Object"); } // Store a placeholder for the number of fields argument. We don't know diff --git a/src/vm/wren_opcodes.h b/src/vm/wren_opcodes.h index 08ccf83c..1626bf22 100644 --- a/src/vm/wren_opcodes.h +++ b/src/vm/wren_opcodes.h @@ -164,13 +164,12 @@ OPCODE(CONSTRUCT) // compiler-generated constructor metaclass methods. OPCODE(FOREIGN_CONSTRUCT) -// Creates a class. Top of stack is the superclass, or `null` if the class -// inherits Object. Below that is a string for the name of the class. Byte -// [arg] is the number of fields in the class. +// Creates a class. Top of stack is the superclass. Below that is a string for +// the name of the class. Byte [arg] is the number of fields in the class. OPCODE(CLASS) -// Creates a foreign class. Top of stack is the superclass, or `null` if the -// class inherits Object. Below that is a string for the name of the class. +// Creates a foreign class. Top of stack is the superclass. Below that is a +// string for the name of the class. OPCODE(FOREIGN_CLASS) // Define a method for symbol [arg]. The class receiving the method is popped diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index d498ce81..5d77958e 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -639,27 +639,21 @@ static bool defineClass(WrenVM* vm, ObjFiber* fiber, int numFields, { // Pull the name and superclass off the stack. Value name = fiber->stackTop[-2]; - Value superclassValue = fiber->stackTop[-1]; + Value superclass = fiber->stackTop[-1]; // We have two values on the stack and we are going to leave one, so discard // the other slot. fiber->stackTop--; - // Use implicit Object superclass if none given. - ObjClass* superclass = vm->objectClass; - - if (!IS_NULL(superclassValue)) + Value error = validateSuperclass(vm, name, superclass, numFields); + if (!IS_NULL(error)) { - Value error = validateSuperclass(vm, name, superclassValue, numFields); - if (!IS_NULL(error)) - { - fiber->stackTop[-1] = error; - return false; - } - superclass = AS_CLASS(superclassValue); + fiber->stackTop[-1] = error; + return false; } - ObjClass* classObj = wrenNewClass(vm, superclass, numFields, AS_STRING(name)); + ObjClass* classObj = wrenNewClass(vm, AS_CLASS(superclass), numFields, + AS_STRING(name)); fiber->stackTop[-1] = OBJ_VAL(classObj); if (numFields == -1) bindForeignClass(vm, classObj, module); diff --git a/test/language/inheritance/inherit_from_null.wren b/test/language/inheritance/inherit_from_null.wren new file mode 100644 index 00000000..85106c06 --- /dev/null +++ b/test/language/inheritance/inherit_from_null.wren @@ -0,0 +1 @@ +class Foo is null {} // expect runtime error: Class 'Foo' cannot inherit from a non-class object.