Bug 148148

Summary: Function.prototype.toString is incorrect for ArrowFunction
Product: WebKit Reporter: Blaze Burg <bburg>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, ggaren, gskachkov, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 140855, 148445    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Blaze Burg
Reported 2015-08-18 15:53:50 PDT
It should be possible to define an arrow function, call toString on it, and evaluate the result without a parse error. TEST CASE: 1 let a = () => {}; 2 a.toString(); // returns function () => {} Evaluating line 2's result will cause a parse error. (Relevant spec: http://people.mozilla.org/~jorendorff/es6-draft.html#sec-function.prototype.tostring)
Attachments
Patch (5.12 KB, patch)
2015-08-19 00:53 PDT, GSkachkov 		no flags
DetailsFormatted DiffDiff
Patch (16.26 KB, patch)
2015-08-19 14:05 PDT, GSkachkov 		no flags
DetailsFormatted DiffDiff
Patch (16.24 KB, patch)
2015-08-19 14:24 PDT, GSkachkov 		no flags
DetailsFormatted DiffDiff
Patch (17.04 KB, patch)
2015-08-25 02:04 PDT, GSkachkov 		no flags
DetailsFormatted DiffDiff
Patch (17.30 KB, patch)
2015-08-25 02:43 PDT, GSkachkov 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
GSkachkov
Comment 1 2015-08-19 00:53:51 PDT
Created attachment 259368 [details] Patch Fix
GSkachkov
Comment 2 2015-08-19 01:50:52 PDT
I've uploaded small patch to fix this issue, but exists one more open issue https://bugs.webkit.org/show_bug.cgi?id=146507 that is related to toString method. I'll fix later
Blaze Burg
Comment 3 2015-08-19 08:20:58 PDT
Comment on attachment 259368 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259368&action=review > Source/JavaScriptCore/ChangeLog:3 > + [ES6] Arrow function syntax. Arrow function specific features. Implement correct toString method This bug title doesn't match the one in the link. > Source/JavaScriptCore/ChangeLog:6 > + Added correct support of toString() method for arrow function Nit: trailing period. > Source/JavaScriptCore/tests/stress/arrowfunction-tostring.js:8 > +var af2 = (a) => {a + 1}; This test is missing several syntax cases. > LayoutTests/ChangeLog:6 > + Added test of toString() method Nit: trailing period.
GSkachkov
Comment 4 2015-08-19 14:05:27 PDT
Created attachment 259403 [details] Patch Fix review comments
GSkachkov
Comment 5 2015-08-19 14:24:44 PDT
Created attachment 259404 [details] Patch Fix review comments & reupload
GSkachkov
Comment 6 2015-08-19 14:26:55 PDT
Comment on attachment 259368 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259368&action=review >> Source/JavaScriptCore/ChangeLog:3 >> + [ES6] Arrow function syntax. Arrow function specific features. Implement correct toString method > > This bug title doesn't match the one in the link. Done >> Source/JavaScriptCore/tests/stress/arrowfunction-tostring.js:8 >> +var af2 = (a) => {a + 1}; > > This test is missing several syntax cases. I've added more tests, but I had to fix some issue in parser, so we need to review it as well
Saam Barati
Comment 7 2015-08-25 00:18:34 PDT
Comment on attachment 259404 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259404&action=review r=me Please reupload with fixed Nit and added test cases for arrow function body expressions that don't end in ";" and I will r+/cq+ > Source/JavaScriptCore/parser/ASTBuilder.h:375 > + SourceCode source = m_sourceCode->subExpression(functionInfo.startOffset, functionInfo.body->isArrowFunctionBodyExpression() ? functionInfo.endOffset -1 : functionInfo.endOffset, functionInfo.startLine, functionInfo.bodyStartColumn); Nit: space between "-" and "1" > Source/JavaScriptCore/parser/ASTBuilder.h:376 > ArrowFuncExprNode* result = new (m_parserArena) ArrowFuncExprNode(location, *functionInfo.name, functionInfo.body, source); The - 1 worries me. Does this work: function foo(x) { print(x.toString() } foo((y)=>y)
GSkachkov
Comment 8 2015-08-25 02:04:15 PDT
Created attachment 259831 [details] Patch Fix review comments & reupload
GSkachkov
Comment 9 2015-08-25 02:43:16 PDT
Created attachment 259833 [details] Patch Add one more test
GSkachkov
Comment 10 2015-08-25 04:46:51 PDT
Comment on attachment 259404 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259404&action=review >> Source/JavaScriptCore/parser/ASTBuilder.h:375 >> + SourceCode source = m_sourceCode->subExpression(functionInfo.startOffset, functionInfo.body->isArrowFunctionBodyExpression() ? functionInfo.endOffset -1 : functionInfo.endOffset, functionInfo.startLine, functionInfo.bodyStartColumn); > > Nit: space between "-" and "1" Done >> Source/JavaScriptCore/parser/ASTBuilder.h:376 >> ArrowFuncExprNode* result = new (m_parserArena) ArrowFuncExprNode(location, *functionInfo.name, functionInfo.body, source); > > The - 1 worries me. > Does this work: > function foo(x) { print(x.toString() } > foo((y)=>y) I've added more tests, check in LayoutTests/js/script-tests/arrowfunction-tostring.js is it enough.
Saam Barati
Comment 11 2015-08-25 11:24:52 PDT
Comment on attachment 259833 [details] Patch r=me
WebKit Commit Bot
Comment 12 2015-08-25 12:10:44 PDT
Comment on attachment 259833 [details] Patch Clearing flags on attachment: 259833 Committed r188928: <http://trac.webkit.org/changeset/188928>
WebKit Commit Bot
Comment 13 2015-08-25 12:10:50 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.