Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for ES6 rest parameter syntax #21

Closed
drsm opened this issue Jun 22, 2018 · 6 comments
Closed

add support for ES6 rest parameter syntax #21

drsm opened this issue Jun 22, 2018 · 6 comments
Assignees

Comments

@drsm
Copy link
Contributor

drsm commented Jun 22, 2018

instead of the arguments object, which is useless in case we going to support other es6+ features.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/rest_parameters

@xeioex
Copy link
Contributor

xeioex commented Dec 20, 2018

Hi @drsm,

Please try the patch below

# HG changeset patch
# User Alexander Pyshchev <yftuls@gmail.com>
# Date 1545073982 -7200
#      Mon Dec 17 21:13:02 2018 +0200
# Node ID 59bd0ebbaa60517ec8269bef11dfa09a6a757881
# Parent  0709c3d38212df011229fccb2e7a9e7f23263cce
Added the rest parameters support.

diff -r 0709c3d38212 -r 59bd0ebbaa60 njs/njs_function.c
--- a/njs/njs_function.c	Fri Dec 07 18:58:27 2018 +0300
+++ b/njs/njs_function.c	Mon Dec 17 21:13:02 2018 +0200
@@ -168,6 +168,40 @@
 
 
+njs_ret_t
+njs_function_rest_parameters_init(njs_vm_t *vm, njs_native_frame_t *frame)
+{
+    nxt_uint_t   nargs, n, i;
+    njs_array_t  *array;
+    njs_value_t  *rest_arguments;
+
+    nargs = frame->nargs;
+    n = frame->function->u.lambda->nargs;
+
+    array = njs_array_alloc(vm, (nargs < n) ? 0 : (nargs - n + 1), 0);
+    if (nxt_slow_path(array == NULL)) {
+        return NXT_ERROR;
+    }
+
+    if (nxt_fast_path(nargs >= n)) {
+        i = 0;
+        while (n <= nargs) {
+            /* GC: retain. */ 
+            array->start[i++] = frame->arguments[n++];
+        }
+    }
+
+    rest_arguments = &frame->arguments[frame->function->u.lambda->nargs];
+
+    /* GC: retain. */
+    rest_arguments->type = NJS_ARRAY;
+    rest_arguments->data.u.array = array;
+    rest_arguments->data.truth = 1;
+
+    return NXT_OK;
+}
+
+
 njs_ret_t
 njs_function_arguments_thrower(njs_vm_t *vm, njs_value_t *value,
     njs_value_t *setval, njs_value_t *retval)
 {
@@ -482,6 +516,13 @@
         }
     }
 
+    if (lambda->rest_parameters) {
+        ret = njs_function_rest_parameters_init(vm, &frame->native);
+        if (nxt_slow_path(ret != NXT_OK)) {
+            return NXT_ERROR;
+        }
+    }
+
     vm->active_frame = frame;
 
     return NJS_APPLIED;
diff -r 0709c3d38212 -r 59bd0ebbaa60 njs/njs_function.h
--- a/njs/njs_function.h	Fri Dec 07 18:58:27 2018 +0300
+++ b/njs/njs_function.h	Mon Dec 17 21:13:02 2018 +0200
@@ -31,6 +31,7 @@
     uint8_t                        block_closures;    /* 4 bits */
 
     uint8_t                        arguments_object;  /* 1 bit */
+    uint8_t                        rest_parameters;   /* 1 bit */
 
     /* Initial values of local scope. */
     njs_value_t                    *local_scope;
@@ -151,6 +152,8 @@
 njs_native_frame_t *njs_function_frame_alloc(njs_vm_t *vm, size_t size);
 njs_ret_t njs_function_arguments_object_init(njs_vm_t *vm,
     njs_native_frame_t *frame);
+njs_ret_t njs_function_rest_parameters_init(njs_vm_t *vm,
+    njs_native_frame_t *frame);
 njs_ret_t njs_function_arguments_thrower(njs_vm_t *vm, njs_value_t *value,
     njs_value_t *setval, njs_value_t *retval);
 njs_ret_t njs_function_prototype_create(njs_vm_t *vm, njs_value_t *value,
diff -r 0709c3d38212 -r 59bd0ebbaa60 njs/njs_lexer.c
--- a/njs/njs_lexer.c	Fri Dec 07 18:58:27 2018 +0300
+++ b/njs/njs_lexer.c	Mon Dec 17 21:13:02 2018 +0200
@@ -326,6 +326,15 @@
         case NJS_TOKEN_DOT:
             p = lexer->start;
 
+            if (((p + 1) < lexer->end)
+                && NJS_TOKEN_DOT == njs_tokens[*(p)]
+                && NJS_TOKEN_DOT == njs_tokens[*(p + 1)])
+            {
+                lexer->text.length = 2 + p - lexer->text.start;
+                lexer->start += 2;
+                return NJS_TOKEN_ELLIPSIS;
+            }
+
             if (p == lexer->end || njs_tokens[*p] != NJS_TOKEN_DIGIT) {
                 lexer->text.length = p - lexer->text.start;
                 return NJS_TOKEN_DOT;
diff -r 0709c3d38212 -r 59bd0ebbaa60 njs/njs_parser.c
--- a/njs/njs_parser.c	Fri Dec 07 18:58:27 2018 +0300
+++ b/njs/njs_parser.c	Mon Dec 17 21:13:02 2018 +0200
@@ -629,6 +629,19 @@
 
     while (token != NJS_TOKEN_CLOSE_PARENTHESIS) {
 
+        if (nxt_slow_path(1 == lambda->rest_parameters)) {
+            return NJS_TOKEN_ILLEGAL;
+        }
+
+        if (nxt_slow_path(token == NJS_TOKEN_ELLIPSIS)) {
+            lambda->rest_parameters = 1;
+
+            token = njs_parser_token(parser);
+            if (nxt_slow_path(token <= NJS_TOKEN_ILLEGAL)) {
+                return NJS_TOKEN_ILLEGAL;
+            }
+        }
+
         if (nxt_slow_path(token != NJS_TOKEN_NAME)) {
             return NJS_TOKEN_ILLEGAL;
         }
diff -r 0709c3d38212 -r 59bd0ebbaa60 njs/njs_parser.h
--- a/njs/njs_parser.h	Fri Dec 07 18:58:27 2018 +0300
+++ b/njs/njs_parser.h	Mon Dec 17 21:13:02 2018 +0200
@@ -29,6 +29,7 @@
 
     NJS_TOKEN_COMMA,
     NJS_TOKEN_DOT,
+    NJS_TOKEN_ELLIPSIS,
     NJS_TOKEN_SEMICOLON,
 
     NJS_TOKEN_COLON,
diff -r 0709c3d38212 -r 59bd0ebbaa60 njs/test/njs_unit_test.c
--- a/njs/test/njs_unit_test.c	Fri Dec 07 18:58:27 2018 +0300
+++ b/njs/test/njs_unit_test.c	Mon Dec 17 21:13:02 2018 +0200
@@ -6063,6 +6063,34 @@
                  "[concat('.',1,2,3), concat('+',1,2,3,4)]"),
       nxt_string("1.2.3,1+2+3+4") },
 
+    { nxt_string("function myFoo(a,b,...other) { return other };"
+                 "myFoo(1,2,3,4,5);" ),
+      nxt_string("3,4,5") },
+
+    { nxt_string("function myFoo(a,b,...other, c) { return other };"),
+      nxt_string("SyntaxError: Unexpected token \"c\" in 1") },
+
+    { nxt_string("function sum(a, b, c, ...other) { return a+b+c+other[2] };"
+                 "sum(\"one \",2,\" three \",\"four \",\"five \",\"the long enough sixth argument \");"),
+      nxt_string("one 2 three the long enough sixth argument ") },
+
+    { nxt_string("function myFoo1(a,...other) { return other };"
+                 "function myFoo2(a,b,...other) { return other };"
+                 "myFoo1(1,2,3,4,5,myFoo2(1,2,3,4));"),
+      nxt_string("2,3,4,5,3,4") },
+
+    { nxt_string("function myFoo(...other) { return (other instanceof Array) };"
+                 "myFoo(1);" ),
+      nxt_string("true") },
+
+    { nxt_string("function myFoo(a,...other) { return other.length };"
+                 "myFoo(1,2,3,4,5);" ),
+      nxt_string("4") },
+
+    { nxt_string("function myFoo(a,b,...other) { return other };"
+                 "myFoo(1,2);" ),
+      nxt_string("") },
+
     /* Scopes. */
 
     { nxt_string("function f(x) { a = x } var a; f(5); a"),

@drsm
Copy link
Contributor Author

drsm commented Dec 20, 2018

Hi @xeioex,
the patch works for me, thanks!
except one minor issue:

>> (function(...xs) {}).length
1
>> (function(x, ...xs) {}).length
2

should be 0, 1 according to the spec:
https://tc39.github.io/ecma262/#sec-functioninitialize
https://tc39.github.io/ecma262/#sec-function-definitions-static-semantics-expectedargumentcount

@drsm
Copy link
Contributor Author

drsm commented Dec 20, 2018

found another one:

>> var v = function(x, ...x) { console.log(x); };
undefined
>> v(1,2,3,4)
[2,3,4]

there should be a SyntaxError.

@xeioex
Copy link
Contributor

xeioex commented Dec 20, 2018

there should be a SyntaxError.

The same is for ordinary functions. The argument' names are not checked for duplicates.

@drsm
Copy link
Contributor Author

drsm commented Dec 20, 2018

The argument' names are not checked for duplicates.

This is allowed in non-strict mode, but not for the rest parameter:

$ node
> var x = function(a,b,a) { console.log(arguments); };
undefined
> x(1,3,4)
[Arguments] { '0': 1, '1': 3, '2': 4 }
undefined
> var x = function(a, ...a) {};
var x = function(a, ...a) {};
                       ^

SyntaxError: Duplicate parameter name not allowed in this context
>

@xeioex
Copy link
Contributor

xeioex commented Dec 20, 2018

We aim to support only the strict mode, so we can consider this as an error even for ordinary functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants