From 689fc77ff1413f7aba4077f1cdc9b5c5b5c330b0 Mon Sep 17 00:00:00 2001 From: hongzhidao Date: Sun, 21 Apr 2019 18:11:58 +0800 Subject: [PATCH] Added block scoped function definitions support. This closes #134 issue on Github. --- njs/njs_parser.c | 1 + njs/njs_variable.c | 77 ++++++++++++++++++------ njs/test/module/declaration_exception.js | 10 +++ njs/test/njs_expect_test.exp | 2 + njs/test/njs_unit_test.c | 46 +++++++++++++- 5 files changed, 117 insertions(+), 19 deletions(-) create mode 100644 njs/test/module/declaration_exception.js diff --git a/njs/njs_parser.c b/njs/njs_parser.c index 54b6c16d..cef217d8 100644 --- a/njs/njs_parser.c +++ b/njs/njs_parser.c @@ -203,6 +203,7 @@ njs_parser_scope_begin(njs_vm_t *vm, njs_parser_t *parser, njs_scope_t type) } else { if (type == NJS_SCOPE_GLOBAL) { type += NJS_INDEX_GLOBAL_OFFSET; + scope->module = vm->options.module; } scope->next_index[0] = type; diff --git a/njs/njs_variable.c b/njs/njs_variable.c index dc6e963b..7b5f38b7 100644 --- a/njs/njs_variable.c +++ b/njs/njs_variable.c @@ -9,6 +9,9 @@ #include +static njs_variable_t *njs_variable_scope_add(njs_vm_t *vm, + njs_parser_scope_t *scope, nxt_lvlhsh_query_t *lhq, + njs_variable_type_t type); static njs_ret_t njs_variable_reference_resolve(njs_vm_t *vm, njs_variable_reference_t *vr, njs_parser_scope_t *node_scope); static njs_variable_t *njs_variable_alloc(njs_vm_t *vm, nxt_str_t *name, @@ -45,7 +48,6 @@ njs_variable_t * njs_variable_add(njs_vm_t *vm, njs_parser_scope_t *scope, nxt_str_t *name, uint32_t hash, njs_variable_type_t type) { - nxt_int_t ret; njs_variable_t *var; nxt_lvlhsh_query_t lhq; @@ -53,36 +55,68 @@ njs_variable_add(njs_vm_t *vm, njs_parser_scope_t *scope, nxt_str_t *name, lhq.key = *name; lhq.proto = &njs_variables_hash_proto; - if (type >= NJS_VARIABLE_VAR) { - /* - * A "var" and "function" declarations are - * stored in function or global scope. - */ - while (scope->type == NJS_SCOPE_BLOCK) { + var = njs_variable_scope_add(vm, scope, &lhq, type); + if (nxt_slow_path(var == NULL)) { + return NULL; + } + + if (type == NJS_VARIABLE_VAR && scope->type == NJS_SCOPE_BLOCK) { + /* A "var" declaration is stored in function or global scope. */ + do { scope = scope->parent; - } + + var = njs_variable_scope_add(vm, scope, &lhq, type); + if (nxt_slow_path(var == NULL)) { + return NULL; + } + + } while (scope->type == NJS_SCOPE_BLOCK); } - if (nxt_lvlhsh_find(&scope->variables, &lhq) == NXT_OK) { - var = lhq.value; + if (type == NJS_VARIABLE_FUNCTION) { + var->type = type; + } + + return var; +} + - if (type == NJS_VARIABLE_FUNCTION) { - var->type = type; +static njs_variable_t * +njs_variable_scope_add(njs_vm_t *vm, njs_parser_scope_t *scope, + nxt_lvlhsh_query_t *lhq, njs_variable_type_t type) +{ + nxt_int_t ret; + njs_variable_t *var; + + if (nxt_lvlhsh_find(&scope->variables, lhq) == NXT_OK) { + var = lhq->value; + + if (!scope->module && scope->type != NJS_SCOPE_BLOCK) { + return var; + } + + if (type == NJS_VARIABLE_FUNCTION + || var->type == NJS_VARIABLE_FUNCTION) + { + njs_parser_syntax_error(vm, vm->parser, + "\"%V\" has already been declared", + &lhq->key); + return NULL; } return var; } - var = njs_variable_alloc(vm, &lhq.key, type); + var = njs_variable_alloc(vm, &lhq->key, type); if (nxt_slow_path(var == NULL)) { - return var; + return NULL; } - lhq.replace = 0; - lhq.value = var; - lhq.pool = vm->mem_pool; + lhq->replace = 0; + lhq->value = var; + lhq->pool = vm->mem_pool; - ret = nxt_lvlhsh_insert(&scope->variables, &lhq); + ret = nxt_lvlhsh_insert(&scope->variables, lhq); if (nxt_fast_path(ret == NXT_OK)) { return var; @@ -459,6 +493,13 @@ njs_variable_reference_resolve(njs_vm_t *vm, njs_variable_reference_t *vr, if (nxt_lvlhsh_find(&scope->variables, &lhq) == NXT_OK) { vr->variable = lhq.value; + if (scope->type == NJS_SCOPE_BLOCK + && vr->variable->type == NJS_VARIABLE_VAR) + { + scope = scope->parent; + continue; + } + if (scope->type == NJS_SCOPE_SHIM) { scope = previous; diff --git a/njs/test/module/declaration_exception.js b/njs/test/module/declaration_exception.js new file mode 100644 index 00000000..518a03d5 --- /dev/null +++ b/njs/test/module/declaration_exception.js @@ -0,0 +1,10 @@ + +function f() { + return 1; +} + +function f() { + return 2; +} + +export default f; diff --git a/njs/test/njs_expect_test.exp b/njs/test/njs_expect_test.exp index bd5adc96..a194735d 100644 --- a/njs/test/njs_expect_test.exp +++ b/njs/test/njs_expect_test.exp @@ -699,6 +699,8 @@ njs_test { "undefined\r\n"} {"import ref from 'ref_exception.js'\r\n" "ReferenceError: \"undeclared\" is not defined in ref_exception.js:1"} + {"import m from 'declaration_exception.js'\r\n" + "SyntaxError: \"f\" has already been declared in declaration_exception.js:6"} {"import m from 'loading_exception.js'\r\n" "Error: loading exception\r\n at module \\(loading_exception.js:1\\)"} {"import lib3 from 'lib1.js'\r\n" diff --git a/njs/test/njs_unit_test.c b/njs/test/njs_unit_test.c index 3ef5dd0f..90ca2b55 100644 --- a/njs/test/njs_unit_test.c +++ b/njs/test/njs_unit_test.c @@ -5746,6 +5746,21 @@ static njs_unit_test_t njs_test[] = { nxt_string("do function f() { } while (0)"), nxt_string("SyntaxError: Functions can only be declared at top level or inside a block in 1") }, + { nxt_string("function f() { return 1; } { function f() { return 2; } } f()"), + nxt_string("1") }, + + { nxt_string("function f() { return 1; } { function f() { return 2; } { function f() { return 3; } }} f()"), + nxt_string("1") }, + + { nxt_string("{ var f; function f() {} }"), + nxt_string("SyntaxError: \"f\" has already been declared in 1") }, + + { nxt_string("{ function f() {} var f; }"), + nxt_string("SyntaxError: \"f\" has already been declared in 1") }, + + { nxt_string("{ function f() {} { var f }}"), + nxt_string("SyntaxError: \"f\" has already been declared in 1") }, + { nxt_string("function f() { return f() } f()"), nxt_string("RangeError: Maximum call stack size exceeded") }, @@ -11998,6 +12013,25 @@ static njs_unit_test_t njs_test[] = }; +static njs_unit_test_t njs_module_test[] = +{ + { nxt_string("function f(){return 2}; var f; f()"), + nxt_string("SyntaxError: \"f\" has already been declared in 1") }, + + { nxt_string("function f(){return 2}; var f = 1; f()"), + nxt_string("SyntaxError: \"f\" has already been declared in 1") }, + + { nxt_string("function f(){return 1}; function f(){return 2}; f()"), + nxt_string("SyntaxError: \"f\" has already been declared in 1") }, + + { nxt_string("var f = 1; function f() {};"), + nxt_string("SyntaxError: \"f\" has already been declared in 1") }, + + { nxt_string("{ var f = 1; } function f() {};"), + nxt_string("SyntaxError: \"f\" has already been declared in 1") }, +}; + + static njs_unit_test_t njs_tz_test[] = { { nxt_string("var d = new Date(1); d = d + ''; d.slice(0, 33)"), @@ -13287,7 +13321,17 @@ main(int argc, char **argv) (void) putenv((char *) "TZ=UTC"); tzset(); - ret = njs_unit_test(njs_test, nxt_nitems(njs_test), 0, + /* script tests. */ + + ret = njs_unit_test(njs_test, nxt_nitems(njs_test), 0, disassemble, + verbose); + if (ret != NXT_OK) { + return ret; + } + + /* module tests. */ + + ret = njs_unit_test(njs_module_test, nxt_nitems(njs_module_test), 1, disassemble, verbose); if (ret != NXT_OK) { return ret; -- 2.47.3