Fixed empty labelled statement in a function.

[VadimZhestikov]

Proposed changes

Fixed empty labelled statement in a function.
It fixes #755.

Checklist

Before creating a PR, run through this checklist and mark each as complete:

• I have read the CONTRIBUTING document
• If applicable, I have added tests that prove my fix is effective or that my feature works
• If applicable, I have checked that any relevant tests pass after adding my changes

[xeioex]

The parser after the patch fails to parse code where empty label is the last statement in the current block

{
v:;
}

do {    
    v:;
} while (false)

Thrown: SyntaxError: Unexpected token "}" in 755.issue.js:4

Also, removing njs_parser_stack_pop() call in the case of semicolon looks wrong to me, because we need to remove a stack entry created by

njs_parser_after(parser, current, (void *) unique_id, 1,                   
                              njs_parser_labelled_statement_after);  

when we end the parsing of a labelled statement. That stack entry is always created by njs_parser_labelled_statement().

Feel free to use the following parser debug: https://gist.github.com/xeioex/ad9976b7fae49ffc4462c2279bed1d86

./configure --addr2line=YES --debug=YES --cc-opt="-O0 -DNJS_PARSER_DEBUG=1" && make njs

With the patch you can see that we call njs_parser_statement_wo_node_labelled() twice, effectively parsing the next statement after the labelled statement as a labelled statement too:

call njs_parser_statement()
set next(labelled_statement)
 after(rser_statement_after, link:0, height:5)
call njs_parser_labelled_statement()
set next(statement_wo_node_labelled)
 after(rser_labelled_statement_after, link:0, height:6)
call njs_parser_statement_wo_node_labelled()
call njs_parser_statement_wo_node_labelled()
set next(expression_statement)
call njs_parser_expression_statement()

Without the patch we have the following log:

call njs_parser_statement()
set next(labelled_statement)
 after(rser_statement_after, link:0, height:5)
call njs_parser_labelled_statement()
set next(statement_wo_node)
 after(rser_labelled_statement_after, link:0, height:6)
call njs_parser_statement_wo_node()
  stack_pop(6): njs_parser_labelled_statement_after()
call njs_parser_labelled_statement_after()
  stack_pop(5): njs_parser_statement_after()
call njs_parser_statement_after()
  stack_pop(4): njs_parser_statement_list_next()
call njs_parser_statement_list_next()

which does the correct unwinding of labelled_statement.

Mentioned issues addressed, patch updated

[xeioex]

The root cause of the problem is that in case of empty labelled statement the AST is missing a part of the tree.

To see that we can compare AST before and after the fix.
To get AST: ./build/njs -a 755.js | yq . -P.

colordiff -u 755.good.ast 755.bad.ast 
--- 755.good.ast        2024-07-09 16:10:46.000000000 -0700
+++ 755.bad.ast 2024-07-09 16:11:40.000000000 -0700
@@ -12,24 +12,6 @@
       left:
         name: STATEMENT
         line: 0
-        left:
-          name: STATEMENT
-          line: 0
-          left:
-            name: STATEMENT
-            line: 0
-            right:
-              name: FUNCTION_DECLARATION
-              line: 3
-              right:
-                name: STATEMENT
-                line: 0
-                right:
-                  name: RETURN
-                  line: 3
-          right:
-            name: BLOCK
-            line: 4
         right:
           name: NUMBER
           line: 5

As a result, inlined function v2() is left underinitialized by generator.

[xeioex]

Instead of changing generic statement functions, I propose to localize the change to
njs_parser_labelled_statement_after(). Now we add empty block ONLY when it is needed, for empty label statement. I also added a couple of additional tests.

commit c89de0f74f3b69e7b3b3ccc8cffc66a1402e7887
Author: Vadim Zhestikov <v.zhestikov@f5.com>
Date:   Fri Jul 5 11:23:33 2024 -0700

    Fixed empty labelled statement in a function.

diff --git a/src/njs_parser.c b/src/njs_parser.c
index dbd9169f..6d584b58 100644
--- a/src/njs_parser.c
+++ b/src/njs_parser.c
@@ -6699,23 +6699,33 @@ njs_parser_labelled_statement_after(njs_parser_t *parser,
 {
     njs_int_t                ret;
     uintptr_t                unique_id;
+    njs_parser_node_t        *node;
     const njs_lexer_entry_t  *entry;
 
-    if (parser->node != NULL) {
-        /* The statement is not empty block or just semicolon. */
-
-        unique_id = (uintptr_t) parser->target;
-        entry = (const njs_lexer_entry_t *) unique_id;
+    node = parser->node;
 
-        ret = njs_name_copy(parser->vm, &parser->node->name, &entry->name);
-        if (ret != NJS_OK) {
+    if (node == NULL) {
+        node = njs_parser_node_new(parser, NJS_TOKEN_BLOCK);
+        if (node == NULL) {
             return NJS_ERROR;
         }
 
-        ret = njs_label_remove(parser->vm, parser->scope, unique_id);
-        if (ret != NJS_OK) {
-            return NJS_ERROR;
-        }
+        node->token_line = token->line;
+
+        parser->node = node;
+    }
+
+    unique_id = (uintptr_t) parser->target;
+    entry = (const njs_lexer_entry_t *) unique_id;
+
+    ret = njs_name_copy(parser->vm, &node->name, &entry->name);
+    if (ret != NJS_OK) {
+        return NJS_ERROR;
+    }
+
+    ret = njs_label_remove(parser->vm, parser->scope, unique_id);
+    if (ret != NJS_OK) {
+        return NJS_ERROR;
     }
 
     return njs_parser_stack_pop(parser);
diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 2f0e318c..75933665 100644
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -3494,6 +3494,22 @@ static njs_unit_test_t  njs_test[] =
                  "} catch(e) {c = 10;}; [c, fin]"),
       njs_str("1,1") },
 
+    { njs_str("function v1() {"
+                 "function v2 () {}"
+                 "v3:;"
+                 "1;"
+              "} v1();"),
+      njs_str("undefined") },
+
+    { njs_str("function v1() {"
+                 "function v2 () {}"
+                 "v3:;"
+              "} v1();"),
+      njs_str("undefined") },
+
+    { njs_str("{v1:;}"),
+      njs_str("undefined") },
+
     /* jumping out of a nested try-catch block. */
 
     { njs_str("var r = 0; "

Patch is updated to address latest comments