From 3eb4ac1419c49e3dcf5f778c02428c47e60dd984 Mon Sep 17 00:00:00 2001 From: underscorediscovery Date: Mon, 30 Sep 2019 20:30:27 -0700 Subject: [PATCH] Add a clearer error message for forward declared lowercase variables (#699) * Add better error messaging for forward referenced top level variables, see #467 * add test case --- src/vm/wren_compiler.c | 18 +++++++++-------- src/vm/wren_core.c | 2 +- src/vm/wren_vm.c | 20 +++++++++++++++---- src/vm/wren_vm.h | 7 +++++-- .../nonlocal/localname_forward_declare.wren | 5 +++++ 5 files changed, 37 insertions(+), 15 deletions(-) create mode 100644 test/language/nonlocal/localname_forward_declare.wren diff --git a/src/vm/wren_compiler.c b/src/vm/wren_compiler.c index 6c90f1ea..caa63534 100644 --- a/src/vm/wren_compiler.c +++ b/src/vm/wren_compiler.c @@ -1253,9 +1253,11 @@ static int declareVariable(Compiler* compiler, Token* token) // Top-level module scope. if (compiler->scopeDepth == -1) { + int line = -1; int symbol = wrenDefineVariable(compiler->parser->vm, compiler->parser->module, - token->start, token->length, NULL_VAL); + token->start, token->length, + NULL_VAL, &line); if (symbol == -1) { @@ -1265,6 +1267,12 @@ static int declareVariable(Compiler* compiler, Token* token) { error(compiler, "Too many module variables defined."); } + else if (symbol == -3) + { + error(compiler, + "Variable '%.*s' referenced before this definition (first use at line %d).", + token->length, token->start, line); + } return symbol; } @@ -2236,13 +2244,7 @@ static void name(Compiler* compiler, bool canAssign) token->start, token->length); if (variable.index == -1) { - if (isLocalName(token->start)) - { - error(compiler, "Undefined variable."); - return; - } - - // If it's a nonlocal name, implicitly define a module-level variable in + // Implicitly define a module-level variable in // the hopes that we get a real definition later. variable.index = wrenDeclareVariable(compiler->parser->vm, compiler->parser->module, diff --git a/src/vm/wren_core.c b/src/vm/wren_core.c index dee77124..34a13c8b 100644 --- a/src/vm/wren_core.c +++ b/src/vm/wren_core.c @@ -1150,7 +1150,7 @@ static ObjClass* defineClass(WrenVM* vm, ObjModule* module, const char* name) ObjClass* classObj = wrenNewSingleClass(vm, 0, nameString); - wrenDefineVariable(vm, module, name, nameString->length, OBJ_VAL(classObj)); + wrenDefineVariable(vm, module, name, nameString->length, OBJ_VAL(classObj), NULL); wrenPopRoot(vm); return classObj; diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index 39bb925c..6ff08e9e 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -456,7 +456,7 @@ static ObjClosure* compileInModule(WrenVM* vm, Value name, const char* source, wrenDefineVariable(vm, module, coreModule->variableNames.data[i]->value, coreModule->variableNames.data[i]->length, - coreModule->variables.data[i]); + coreModule->variables.data[i], NULL); } } @@ -1498,8 +1498,15 @@ int wrenDeclareVariable(WrenVM* vm, ObjModule* module, const char* name, return wrenSymbolTableAdd(vm, &module->variableNames, name, length); } +// Returns `true` if [name] is a local variable name (starts with a lowercase +// letter). +static bool isLocalName(const char* name) +{ + return name[0] >= 'a' && name[0] <= 'z'; +} + int wrenDefineVariable(WrenVM* vm, ObjModule* module, const char* name, - size_t length, Value value) + size_t length, Value value, int* line) { if (module->variables.count == MAX_MODULE_VARS) return -2; @@ -1516,9 +1523,14 @@ int wrenDefineVariable(WrenVM* vm, ObjModule* module, const char* name, } else if (IS_NUM(module->variables.data[symbol])) { - // An implicitly declared variable's value will always be a number. Now we - // have a real definition. + // An implicitly declared variable's value will always be a number. + // Now we have a real definition. + if(line) *line = AS_NUM(module->variables.data[symbol]); module->variables.data[symbol] = value; + + // If this was a localname we want to error if it was + // referenced before this definition. + if (isLocalName(name)) symbol = -3; } else { diff --git a/src/vm/wren_vm.h b/src/vm/wren_vm.h index 4e30b326..ccb32f13 100644 --- a/src/vm/wren_vm.h +++ b/src/vm/wren_vm.h @@ -163,12 +163,15 @@ Value wrenFindVariable(WrenVM* vm, ObjModule* module, const char* name); int wrenDeclareVariable(WrenVM* vm, ObjModule* module, const char* name, size_t length, int line); -// Adds a new top-level variable named [name] to [module]. +// Adds a new top-level variable named [name] to [module], and optionally +// populates line with the line of the implicit first use (line can be NULL). // // Returns the symbol for the new variable, -1 if a variable with the given name // is already defined, or -2 if there are too many variables defined. +// Returns -3 if this is a top-level lowercase variable (localname) that was +// used before being defined. int wrenDefineVariable(WrenVM* vm, ObjModule* module, const char* name, - size_t length, Value value); + size_t length, Value value, int* line); // Pushes [closure] onto [fiber]'s callstack to invoke it. Expects [numArgs] // arguments (including the receiver) to be on the top of the stack already. diff --git a/test/language/nonlocal/localname_forward_declare.wren b/test/language/nonlocal/localname_forward_declare.wren new file mode 100644 index 00000000..e872096c --- /dev/null +++ b/test/language/nonlocal/localname_forward_declare.wren @@ -0,0 +1,5 @@ +if (false) { + System.print(a) +} + +var a = 123 // expect error: Error at '123': Variable 'a' referenced before this definition (first use at line 2). \ No newline at end of file