RESOLVED FIXED 148445
[ES6] Arrow function syntax. Add missed tests for invoking the arrow function that created by 'eval' statement.
https://bugs.webkit.org/show_bug.cgi?id=148445
Summary [ES6] Arrow function syntax. Add missed tests for invoking the arrow function...
GSkachkov
Reported 2015-08-25 13:36:55 PDT
eval('var x=x=>x+10'); x(111); // Returns Exception: SyntaxError: Invalid character: '\0'
Attachments
Patch (2.06 KB, patch)
2015-08-25 13:59 PDT, GSkachkov
no flags
Patch (2.88 KB, patch)
2015-08-31 12:31 PDT, GSkachkov
no flags
GSkachkov
Comment 1 2015-08-25 13:59:21 PDT
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 2 2015-08-25 19:32:46 PDT
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
Comment 3 2015-08-26 09:10:12 PDT
(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
Comment 4 2015-08-27 16:57:19 PDT
(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
Comment 5 2015-08-31 12:31:22 PDT
Created attachment 260307 [details] Patch Updated patch
Yusuke Suzuki
Comment 6 2015-08-31 14:41:08 PDT
Comment on attachment 260307 [details] Patch r=me
WebKit Commit Bot
Comment 7 2015-08-31 15:29:54 PDT
Comment on attachment 260307 [details] Patch Clearing flags on attachment: 260307 Committed r189187: <http://trac.webkit.org/changeset/189187>
WebKit Commit Bot
Comment 8 2015-08-31 15:29:58 PDT
All reviewed patches have been landed. Closing bug.
GSkachkov
Comment 9 2015-08-31 23:44:09 PDT
(In reply to comment #6) > Comment on attachment 260307 [details] > Patch > > r=me Thanks!
Note You need to log in before you can comment on or make changes to this bug.