Summary: | Function.prototype.toString is incorrect for ArrowFunction | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||||||||||
Component: | JavaScriptCore | Assignee: | 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
BJ Burg
2015-08-18 15:53:50 PDT
Created attachment 259368 [details]
Patch
Fix
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 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. Created attachment 259403 [details]
Patch
Fix review comments
Created attachment 259404 [details]
Patch
Fix review comments & reupload
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 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) Created attachment 259831 [details]
Patch
Fix review comments & reupload
Created attachment 259833 [details]
Patch
Add one more test
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. Comment on attachment 259833 [details]
Patch
r=me
Comment on attachment 259833 [details] Patch Clearing flags on attachment: 259833 Committed r188928: <http://trac.webkit.org/changeset/188928> All reviewed patches have been landed. Closing bug. |