Bug 148445

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

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?

(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?

(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 :)

Created attachment 260307 [details] Patch Updated patch

Comment on attachment 260307 [details] Patch r=me

Comment on attachment 260307 [details] Patch Clearing flags on attachment: 260307 Committed r189187: <http://trac.webkit.org/changeset/189187>

All reviewed patches have been landed. Closing bug.

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