Bug 148445 – [ES6] Arrow function syntax. Add missed tests for invoking the arrow function that created by 'eval' statement.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[GSkachkov]

eval('var x=x=>x+10');
x(111); // Returns Exception: SyntaxError: Invalid character: '\0'

[GSkachkov]

Created attachment 259876 [details]
Patch

Added test for this case. Issue was fixed by following patch https://bugs.webkit.org/show_bug.cgi?id=148148

[Yusuke Suzuki]

Comment on attachment 259876 [details]
Patch

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

> LayoutTests/ChangeLog:5
> +

Could you tell me which part fixed this issue in the original patch? http://trac.webkit.org/changeset/188928
Describing it here is nice.

And could you tell me why `functionInfo.endOffset - 1` is needed in the original patch?

[GSkachkov]

(In reply to comment #2)
> Comment on attachment 259876 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=259876&action=review
> 
> > LayoutTests/ChangeLog:5
> > +
> 
> Could you tell me which part fixed this issue in the original patch?
All changes, that were made in 'parser' module, fix current issue. 
> http://trac.webkit.org/changeset/188928
> Describing it here is nice.
> 
> And could you tell me why `functionInfo.endOffset - 1` is needed in the
> original patch?
The issue was that during first parsing arrow function, with body as expression (z=>z*2), has redundant symbol at the end, i.e. var f = x => x+1; and after parsing it has own source as 'x => x+1;' with redundant ';' at the end. It is OK for all cases except case for eval, when it leads to error during invoking function. I found this issue when was fixing issue with toString method, because toString returns extra ';' but should not. So I decided to add test that will prevent this issue with eval in future. 
 
Possible it would be better to split original patch into two patches, 'fix parsing' and add 'correct toString' to avoid misunderstanding, also possible I need to rename current issue to just 'adding additional tests for arrow function'.

What do you think I need to do to make this issue more clear?

[Yusuke Suzuki]

(In reply to comment #3)
> The issue was that during first parsing arrow function, with body as
> expression (z=>z*2), has redundant symbol at the end, i.e. var f = x => x+1;
> and after parsing it has own source as 'x => x+1;' with redundant ';' at the
> end. It is OK for all cases except case for eval, when it leads to error
> during invoking function. I found this issue when was fixing issue with
> toString method, because toString returns extra ';' but should not. So I
> decided to add test that will prevent this issue with eval in future. 
>  
> Possible it would be better to split original patch into two patches, 'fix
> parsing' and add 'correct toString' to avoid misunderstanding, also possible
> I need to rename current issue to just 'adding additional tests for arrow
> function'.
> 
> What do you think I need to do to make this issue more clear?

Yeah. I think describing the details in ChangeLog (and issue title) is preferable :)

[GSkachkov]

Created attachment 260307 [details]
Patch

Updated patch

[Yusuke Suzuki]

Comment on attachment 260307 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 260307 [details]
Patch

Clearing flags on attachment: 260307

Committed r189187: <http://trac.webkit.org/changeset/189187>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[GSkachkov]

(In reply to comment #6)
> Comment on attachment 260307 [details]
> Patch
> 
> r=me

Thanks!