Generator: select a for-in route by the target's own token type

Previously, njs_generate_for_in_statement() worked out what a for-in header
declared from the shape of the tree: a declaration was recognised as a
STATEMENT wrapper whose right child held the kind, and the kind and the name
were reached through that wrapper.  The wrapper was navigated again in the
object and body states, and njs_generate_for_let_update() walked it to find
lexical bindings.  Nothing named the contract, and the property route used
the same "foreach->left->right" to mean the property key.

The fix is to let the parser put the declaration node under the "in" node
directly, so the target's token type says what it is: VAR, LET or CONST for a
declaration, PROPERTY or PROPERTY_REF for a member target, and NAME for a
plain one.  An unrecognised target is now an internal error instead of
falling into the plain-name route.  njs_generate_let_update() takes the
single declaration a for-in header has, and njs_generate_for_let_update()
keeps the list walk the classic "for" needs.

The instruction stream is unchanged.  The code map for PROPERTY_NEXT on the
declaration route now carries the line of the declaration instead of the zero
the removed wrapper held; backtraces are unaffected.

diff --git a/src/njs_generator.c b/src/njs_generator.c
index fc8e787623ac36d540b39534940216422d982bf3..37ecea0834625bd548a065ee3edb49d73004add8 100644 (file)
--- a/src/njs_generator.c
+++ b/src/njs_generator.c
@@ -117,6 +117,15 @@ njs_generate_is_property_lvalue(njs_parser_node_t *node)
 }
 
 
+static njs_bool_t
+njs_generate_is_declaration(njs_parser_node_t *node)
+{
+    return node->token_type == NJS_TOKEN_VAR
+           || node->token_type == NJS_TOKEN_LET
+           || node->token_type == NJS_TOKEN_CONST;
+}
+
+
 njs_inline njs_bool_t
 njs_generate_is_property_call_source(njs_parser_node_t *node)
 {
@@ -2083,28 +2092,40 @@ njs_generate_for_end(njs_vm_t *vm, njs_generator_t *generator,
 
 
 static njs_int_t
-njs_generate_for_let_update(njs_vm_t *vm, njs_generator_t *generator,
-    njs_parser_node_t *node)
+njs_generate_let_update(njs_vm_t *vm, njs_generator_t *generator,
+    njs_parser_node_t *let)
 {
-    njs_parser_node_t         *let;
     njs_vmcode_variable_t     *code_var;
     njs_variable_reference_t  *ref;
 
-    while (node != NULL && node->token_type == NJS_TOKEN_STATEMENT) {
-        let = node->right;
+    if (let->token_type != NJS_TOKEN_LET
+        && let->token_type != NJS_TOKEN_CONST)
+    {
+        return NJS_DECLINED;
+    }
 
-        if (let->token_type != NJS_TOKEN_LET
-            && let->token_type != NJS_TOKEN_CONST)
-        {
-            return NJS_OK;
-        }
+    ref = &let->left->u.reference;
+
+    if (ref->variable->closure) {
+        njs_generate_code(generator, njs_vmcode_variable_t, code_var,
+                          NJS_VMCODE_LET_UPDATE, let);
+        code_var->dst = let->left->index;
+    }
+
+    return NJS_OK;
+}
 
-        ref = &let->left->u.reference;
 
-        if (ref->variable->closure) {
-            njs_generate_code(generator, njs_vmcode_variable_t, code_var,
-                              NJS_VMCODE_LET_UPDATE, let);
-            code_var->dst = let->left->index;
+static njs_int_t
+njs_generate_for_let_update(njs_vm_t *vm, njs_generator_t *generator,
+    njs_parser_node_t *node)
+{
+    njs_int_t  ret;
+
+    while (node != NULL && node->token_type == NJS_TOKEN_STATEMENT) {
+        ret = njs_generate_let_update(vm, generator, node->right);
+        if (ret != NJS_OK) {
+            return (ret == NJS_DECLINED) ? NJS_OK : ret;
         }
 
         node = node->left;
@@ -2201,18 +2222,13 @@ njs_generate_for_in_object_wo_decl(njs_vm_t *vm, njs_generator_t *generator,
     njs_parser_node_t *node)
 {
     njs_int_t                  ret;
-    njs_parser_node_t          *foreach, *name;
+    njs_parser_node_t          *foreach;
     njs_generator_loop_ctx_t   *ctx;
     njs_vmcode_prop_foreach_t  *prop_foreach;
 
     ctx = generator->context;
 
     foreach = node->left;
-    name = foreach->left->right;
-
-    if (name != NULL) {
-        ctx->var->init = 1;
-    }
 
     njs_generate_code(generator, njs_vmcode_prop_foreach_t, prop_foreach,
                       NJS_VMCODE_PROPERTY_FOREACH, foreach);
@@ -2280,29 +2296,25 @@ njs_generate_for_in_statement(njs_vm_t *vm, njs_generator_t *generator,
 
     foreach = node->left;
 
-    if (!njs_generate_is_property_lvalue(foreach->left)) {
-        name = foreach->left->right;
+    if (njs_generate_is_declaration(foreach->left)) {
+        name = foreach->left->left;
 
-        if (name != NULL) {
-            name = name->left;
-
-            ret = njs_generate_variable_wo_dest(vm, generator, name,
+        ret = njs_generate_variable_wo_dest(vm, generator, name,
                                             NJS_DECLARATION, &ctx.var);
-            if (njs_slow_path(ret != NJS_OK)) {
-                return NJS_ERROR;
-            }
+        if (njs_slow_path(ret != NJS_OK)) {
+            return NJS_ERROR;
+        }
 
-            ctx.index_next_value = name->index;
+        ctx.index_next_value = name->index;
 
-            njs_generator_next(generator, njs_generate, foreach->right);
+        njs_generator_next(generator, njs_generate, foreach->right);
 
-            return njs_generator_after(vm, generator,
-                                       njs_queue_first(&generator->stack), node,
-                                       njs_generate_for_in_object,
-                                       &ctx, sizeof(njs_generator_loop_ctx_t));
-        }
+        return njs_generator_after(vm, generator,
+                                   njs_queue_first(&generator->stack), node,
+                                   njs_generate_for_in_object,
+                                   &ctx, sizeof(njs_generator_loop_ctx_t));
 
-    } else {
+    } else if (njs_generate_is_property_lvalue(foreach->left)) {
 
         /* foreach->right is object in 'in object'. */
 
@@ -2313,14 +2325,19 @@ njs_generate_for_in_statement(njs_vm_t *vm, njs_generator_t *generator,
                                    njs_generate_for_in_object_left_hand_expr,
                                    &ctx, sizeof(njs_generator_loop_ctx_t));
 
+    } else if (foreach->left->token_type == NJS_TOKEN_NAME) {
+
+        njs_generator_next(generator, njs_generate, foreach->right);
+
+        return njs_generator_after(vm, generator,
+                                   njs_queue_first(&generator->stack), node,
+                                   njs_generate_for_in_object_wo_decl,
+                                   &ctx, sizeof(njs_generator_loop_ctx_t));
     }
 
-    njs_generator_next(generator, njs_generate, foreach->right);
+    njs_internal_error(vm, "unexpected for-in target");
 
-    return  njs_generator_after(vm, generator,
-                              njs_queue_first(&generator->stack), node,
-                              njs_generate_for_in_object_wo_decl,
-                              &ctx, sizeof(njs_generator_loop_ctx_t));
+    return NJS_ERROR;
 }
 
 
@@ -2427,18 +2444,14 @@ static njs_int_t
 njs_generate_for_in_object(njs_vm_t *vm, njs_generator_t *generator,
     njs_parser_node_t *node)
 {
-    njs_parser_node_t          *foreach, *name;
+    njs_parser_node_t          *foreach;
     njs_generator_loop_ctx_t   *ctx;
     njs_vmcode_prop_foreach_t  *prop_foreach;
 
     ctx = generator->context;
 
     foreach = node->left;
-    name = foreach->left->right;
-
-    if (name != NULL) {
-        ctx->var->init = 1;
-    }
+    ctx->var->init = 1;
 
     njs_generate_code(generator, njs_vmcode_prop_foreach_t, prop_foreach,
                       NJS_VMCODE_PROPERTY_FOREACH, foreach);
@@ -2518,20 +2531,19 @@ njs_generate_for_in_body(njs_vm_t *vm, njs_generator_t *generator,
 {
     njs_int_t                 ret;
     njs_jump_off_t            prop_offset;
-    njs_parser_node_t         *foreach, *name;
+    njs_parser_node_t         *foreach;
     njs_vmcode_prop_next_t    *prop_next;
     njs_generator_loop_ctx_t  *ctx;
 
     ctx = generator->context;
 
     foreach = node->left;
-    name = foreach->left->right;
 
     /* The loop iterator. */
 
-    if (name != NULL) {
-        ret = njs_generate_for_let_update(vm, generator, foreach->left);
-        if (njs_slow_path(ret != NJS_OK)) {
+    if (njs_generate_is_declaration(foreach->left)) {
+        ret = njs_generate_let_update(vm, generator, foreach->left);
+        if (njs_slow_path(ret != NJS_OK && ret != NJS_DECLINED)) {
             return ret;
         }
     }

diff --git a/src/njs_parser.c b/src/njs_parser.c
index 63f6badba6d929620a468c7a5e752d352b737503..661728eef5f22fb9d3e13eeecb6de7484d3d248c 100644 (file)
--- a/src/njs_parser.c
+++ b/src/njs_parser.c
@@ -5815,7 +5815,7 @@ njs_parser_for_var_in_of_expression(njs_parser_t *parser,
             }
 
             node->token_line = token->line;
-            node->left = parser->node;
+            node->left = parser->node->right;
 
         } else {
             if (!njs_parser_is_lvalue(parser->node)) {

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index ccf323a28719a4e50eba63b1c7aedae2814d2802..4d104d949fedd1dbcbfa86cf321cca8645301e9b 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -3500,6 +3500,13 @@ static njs_unit_test_t  njs_test[] =
               "s + o.p"),
       njs_str("abb") },
 
+    { njs_str("var o = [], s = ''; for (o[0] in {a:1, b:2}) {s += o[0]};"
+              "s + o[0]"),
+      njs_str("abb") },
+
+    { njs_str("var k, s = ''; for ((k) in {a:1, b:2}) {s += k}; s + k"),
+      njs_str("abb") },
+
     { njs_str("var s = 'x'; for (let k in {}) {s = 'y'}; s"),
       njs_str("x") },