diff --git a/src/compiler.c b/src/compiler.c index a49e2bfd..3729b622 100644 --- a/src/compiler.c +++ b/src/compiler.c @@ -8,6 +8,13 @@ #define MAX_NAME 256 +// TODO(bob): Are these really worth the effort? +#define PUSH_SCOPE \ + Scope scope##__LINE__; \ + pushScope(compiler, &scope##__LINE__); + +#define POP_SCOPE popScope(compiler) + typedef enum { TOKEN_LEFT_PAREN, @@ -100,6 +107,18 @@ typedef struct int hasError; } Parser; +typedef struct sScope +{ + // The number of previously defined local variables when this scope was + // created. Used to know how many variables to discard when this scope is + // exited. + int previousLocals; + + // The scope enclosing this one, or NULL if this is the top scope in the + // function. + struct sScope* parent; +} Scope; + typedef struct sCompiler { Parser* parser; @@ -117,6 +136,9 @@ typedef struct sCompiler // Non-zero if the function being compiled is a method. int isMethod; + + // The current local variable scope. Initially NULL. + Scope* scope; } Compiler; // Initializes [compiler]. @@ -132,6 +154,8 @@ static void initCompiler(Compiler* compiler, Parser* parser, compiler->fn = newFunction(parser->vm); compiler->fn->numConstants = 0; + + compiler->scope = NULL; } // Outputs a compile or syntax error. @@ -558,16 +582,17 @@ static int declareVariable(Compiler* compiler) consume(compiler, TOKEN_NAME, "Expected variable name."); SymbolTable* symbols; - if (compiler->parent) - { - // Nested block, so this is a local variable. - symbols = &compiler->locals; - } - else + // The top-level scope of the top-level compiler is global scope. + if (compiler->parent == NULL && compiler->scope == NULL) { // Top level global variable. symbols = &compiler->parser->vm->globalSymbols; } + else + { + // Nested block, so this is a local variable. + symbols = &compiler->locals; + } int symbol = addSymbol(symbols, compiler->parser->source + compiler->parser->previous.start, @@ -576,6 +601,7 @@ static int declareVariable(Compiler* compiler) if (symbol == -1) { error(compiler, "Variable is already defined."); + // TODO(bob): Need to allow shadowing for local variables. } return symbol; @@ -584,9 +610,8 @@ static int declareVariable(Compiler* compiler) // Stores a variable with the previously defined symbol in the current scope. static void defineVariable(Compiler* compiler, int symbol) { - // If it's a global variable, we need to explicitly store it. If it's a local - // variable, the value is already on the stack in the right slot. - if (!compiler->parent) + // The top-level scope of the top-level compiler is global scope. + if (compiler->parent == NULL && compiler->scope == NULL) { // It's a global variable, so store the value in the global slot. emit(compiler, CODE_STORE_GLOBAL); @@ -609,7 +634,7 @@ static void defineVariable(Compiler* compiler, int symbol) // CODE_POP // discard previous result in sequence // // - // Would be good to either peephole optimize this or be smarted about + // Would be good to either peephole optimize this or be smarter about // generating code for defining local variables to not emit the DUP // sometimes. emit(compiler, CODE_DUP); @@ -635,6 +660,7 @@ typedef enum // Forward declarations since the grammar is recursive. static void expression(Compiler* compiler, int allowAssignment); +static void assignment(Compiler* compiler); static void statement(Compiler* compiler); static void definition(Compiler* compiler); static void parsePrecedence(Compiler* compiler, int allowAssignment, @@ -676,7 +702,7 @@ static void parameterList(Compiler* compiler, char* name, int* length) static void grouping(Compiler* compiler, int allowAssignment) { - expression(compiler, 0); + assignment(compiler); consume(compiler, TOKEN_RIGHT_PAREN, "Expect ')' after expression."); } @@ -1090,6 +1116,33 @@ static void patchJump(Compiler* compiler, int offset) compiler->fn->bytecode[offset] = compiler->numCodes - offset - 1; } +// Starts a new local block scope. +static void pushScope(Compiler* compiler, Scope* scope) +{ + scope->parent = compiler->scope; + scope->previousLocals = compiler->locals.count; + compiler->scope = scope; +} + +// Closes the last pushed block scope. +static void popScope(Compiler* compiler) +{ + ASSERT(compiler->scope != NULL, "Cannot pop top-level scope."); + + Scope* scope = compiler->scope; + + // Pop locals off the stack. + // TODO(bob): Could make a single instruction that pops multiple values if + // this is a bottleneck. + for (int i = scope->previousLocals; i < compiler->locals.count; i++) + { + emit(compiler, CODE_POP); + } + + truncateSymbolTable(&compiler->locals, scope->previousLocals); + compiler->scope = scope->parent; +} + // Parses a "statement": any expression including expressions like variable // declarations which can only appear at the top level of a block. void statement(Compiler* compiler) @@ -1107,7 +1160,9 @@ void statement(Compiler* compiler) int ifJump = emit(compiler, 255); // Compile the then branch. - statement(compiler); + PUSH_SCOPE; + definition(compiler); + POP_SCOPE; // Jump over the else branch when the if branch is taken. emit(compiler, CODE_JUMP); @@ -1118,7 +1173,9 @@ void statement(Compiler* compiler) // Compile the else branch if there is one. if (match(compiler, TOKEN_ELSE)) { - statement(compiler); + PUSH_SCOPE; + definition(compiler); + POP_SCOPE; } else { @@ -1145,7 +1202,9 @@ void statement(Compiler* compiler) int exitJump = emit(compiler, 255); // Compile the body. - statement(compiler); + PUSH_SCOPE; + definition(compiler); + POP_SCOPE; // Loop back to the top. emit(compiler, CODE_LOOP); @@ -1163,6 +1222,7 @@ void statement(Compiler* compiler) // Curly block. if (match(compiler, TOKEN_LEFT_BRACE)) { + PUSH_SCOPE; // TODO(bob): This code is duplicated in fn, method and top level parsing. for (;;) { @@ -1181,6 +1241,8 @@ void statement(Compiler* compiler) // Discard the result of the previous expression. emit(compiler, CODE_POP); } + + POP_SCOPE; return; } @@ -1299,6 +1361,7 @@ void definition(Compiler* compiler) if (match(compiler, TOKEN_VAR)) { + // TODO(bob): Variable should not be in scope until after initializer. int symbol = declareVariable(compiler); // TODO(bob): Allow uninitialized vars? diff --git a/src/vm.c b/src/vm.c index 8e2ce41b..78b794d8 100644 --- a/src/vm.c +++ b/src/vm.c @@ -365,6 +365,16 @@ void clearSymbolTable(SymbolTable* symbols) } } +void truncateSymbolTable(SymbolTable* symbols, int count) +{ + ASSERT(count <= symbols->count, "Cannot truncate to larger size."); + for (int i = count; i < symbols->count; i++) + { + free(symbols->names[i]); + } + symbols->count = count; +} + int addSymbolUnchecked(SymbolTable* symbols, const char* name, size_t length) { // TODO(bob): Get rid of explicit malloc here. diff --git a/src/vm.h b/src/vm.h index 80caac3c..901d77cf 100644 --- a/src/vm.h +++ b/src/vm.h @@ -178,6 +178,9 @@ Value newString(VM* vm, const char* text, size_t length); // Initializes the symbol table. void initSymbolTable(SymbolTable* symbols); +// Removes any symbols added after [count] symbols were defined. +void truncateSymbolTable(SymbolTable* symbols, int count); + // Frees all dynamically allocated memory used by the symbol table, but not the // SymbolTable itself. void clearSymbolTable(SymbolTable* symbols); diff --git a/test/if.wren b/test/if.wren index 7669350a..f45b5cd3 100644 --- a/test/if.wren +++ b/test/if.wren @@ -36,3 +36,11 @@ if // Newline after "else". if (false) io.write("bad") else io.write("good") // expect: good + +// Definition in then arm. +if (true) var a = io.write("ok") // expect: ok +if (true) class Foo {} // no error + +// Definition in else arm. +if (false) null else var a = io.write("ok") // expect: ok +if (true) null else class Foo {} // no error diff --git a/test/scope_if.wren b/test/scope_if.wren new file mode 100644 index 00000000..0c49ba9d --- /dev/null +++ b/test/scope_if.wren @@ -0,0 +1,9 @@ +// Create a local scope for the 'then' expression. +var a = "out" +if (true) var a = "in" +io.write(a) // expect: out + +// Create a local scope for the 'else' expression. +var b = "out" +if (false) "dummy" else var b = "in" +io.write(b) // expect: out diff --git a/test/scope_reuse_in_different_blocks.wren b/test/scope_reuse_in_different_blocks.wren new file mode 100644 index 00000000..16aa990b --- /dev/null +++ b/test/scope_reuse_in_different_blocks.wren @@ -0,0 +1,9 @@ +{ + var a = "first" + io.write(a) // expect: first +} + +{ + var a = "second" + io.write(a) // expect: second +} diff --git a/test/scope_while.wren b/test/scope_while.wren new file mode 100644 index 00000000..4eb2b9be --- /dev/null +++ b/test/scope_while.wren @@ -0,0 +1,7 @@ +// Body has its own scope. +var a = "outer" +var i = 0 +while ((i = i + 1) <= 1) var a = "inner" +io.write(a) // expect: outer + +// TODO(bob): What about condition? diff --git a/test/while.wren b/test/while.wren index 283a5095..d8329e63 100644 --- a/test/while.wren +++ b/test/while.wren @@ -29,3 +29,7 @@ var f = while (e < 3) { e = e + 1 } io.write(f) // expect: null + +// Definition body. +while (false) var a = "ok" // no error +while (false) class Foo {} // no error