[Webkit-unassigned] [Bug 146934] [ES6] Arrow function syntax. Arrow function should support the destructuring parameters.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=146934

Saam Barati <sbarati at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #268861|review?                     |review-
              Flags|                            |

--- Comment #6 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 268861
  --> https://bugs.webkit.org/attachment.cgi?id=268861
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=268861&action=review

this seems like the right approach, just a few comments to clean up the code.

> Source/JavaScriptCore/parser/Parser.h:1047
> +    ALWAYS_INLINE bool isArrowFunctionNoOrOneParamter()
>      {
>          bool isArrowFunction = false;
>          
> -        if (match(EOFTOK))
> +        if (match(EOFTOK) || (!match(IDENT) && !match(OPENPAREN)))
>              return isArrowFunction;
>          
>          SavePoint saveArrowFunctionPoint = createSavePoint();
> +        if (match(IDENT))
> +            isArrowFunction = consume(IDENT) && match(ARROWFUNCTION);
> +        else if (consume(OPENPAREN) && consume(CLOSEPAREN))
> +            isArrowFunction = match(ARROWFUNCTION);
>          
> -        if (consume(OPENPAREN)) {
> -            bool isArrowFunctionParamters = true;
> +        restoreSavePoint(saveArrowFunctionPoint);
> +        
> +        return isArrowFunction;
> +    }

This function has some redundancy with the below function, lets just combine
the necessary bits into the below function.

> Source/JavaScriptCore/parser/Parser.h:1049
> +    template <class TreeBuilder> bool isArrowFunctionParamters(TreeBuilder& context)

We don't need to templatize this once we move it to SyntaxChecker (see below).
Also, this function has been misspelled, lets fix the spelling in this patch:
"isArrowFunctionParamters" => "isArrowFunctionParameters"

> Source/JavaScriptCore/parser/Parser.h:1057
> +        if (isArrowFunctionNoOrOneParamter())
> +            isArrowFunction = true;

Instead of calling the above function, you
can just check:
```
if (match(IDENT)) {
   save point create;
   next();
   check if arrow function
   save point restore;
} else if ....
```

> Source/JavaScriptCore/parser/Parser.h:1066
> +            unsigned parametersCount = 1;

start this at zero.
Even though we ignore this number, it should start at zero.

> Source/JavaScriptCore/parser/Parser.h:1067
> +            isArrowFunction = parseFormalParameters(context, context.createFormalParameterList(), parametersCount) && consume(CLOSEPAREN) && match(ARROWFUNCTION);

Instead of passing in "context" here, we should create a SyntaxChecker and pass that in.
That way we're not actually allocating AST nodes that we throw away.

> LayoutTests/js/script-tests/arrowfunction-syntax.js:79
>  

add a test or a few tests for rest parameters:
```
(...rest) => rest;
```
etc.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160113/04d597a1/attachment.html>