* SUMMARY Improve Type Profiler Support for ES6 Syntax. • ScriptSyntaxTree should support ES6 syntax that Esprima produces - template strings - modules, packages (export/import) - arrow functions - binding patterns - comprehensions? (ES7?) • BasicBlockAnnotator for method syntax is a bit off - closing brace doesn't appear to be included
<rdar://problem/20335738>
Retitling to be specifically about Arrow Functions. I think we've handled the rest for ES6, and Block Annotations is aa separate issue.
Created attachment 261768 [details] patch
Attachment 261768 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/TypeProfiler.h:107: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 261768 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=261768&action=review > Source/JavaScriptCore/runtime/TypeProfiler.h:73 > + } > + bool operator==(const QueryKey& other) const Style: Normally there are newlines between multi-line member functions. > Source/JavaScriptCore/tests/typeProfiler/arrow-functions.js:4 > +let foo = (x) => x; > +let bar = abc => abc; Might want to also test a case with {}s. let baz = x => { return x } > Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js:152 > + console.assert( node.type === WebInspector.ScriptSyntaxTree.NodeType.FunctionDeclaration || node.type === WebInspector.ScriptSyntaxTree.NodeType.FunctionExpression || node.type === WebInspector.ScriptSyntaxTree.NodeType.MethodDefinition || node.type === WebInspector.ScriptSyntaxTree.NodeType.ArrowFunctionExpression); Style: Unnecessary leading space inside the assert(...). > Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js:372 > + case WebInspector.ScriptSyntaxTree.NodeType.ArrowFunctionExpression: > callback(node, state); > this._recurse(node.id, callback, state); > this._recurseArray(node.params, callback, state); r- ArrowFunctionExpression has "expression" but FunctionExpression/FunctionDeclaration don't. We'll need to walk over that. Either a new case, or check `if (node.expression)`. > Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js:955 > + ArrowFunctionExpression: Symbol("arrow-function-expression"), Nit: Alphabetical order (`sort`) We should add a basic test for the frontend's new node type in: LayoutTests/inspector/model/parse-script-syntax-tree.html
Comment on attachment 261768 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=261768&action=review >> Source/JavaScriptCore/tests/typeProfiler/arrow-functions.js:4 >> +let bar = abc => abc; > > Might want to also test a case with {}s. > > let baz = x => { return x } And maybe even an empty case? () => {}
Created attachment 261782 [details] patch Fixed comments. Also noted in comments that the "expression" field is just a boolean. Not an AST node.
Comment on attachment 261782 [details] patch r=me!
landed in: http://trac.webkit.org/changeset/190146