Currently, when the parser is parsing a variable declaration list and sees a var declarator without an initializer, it just adds the variable name to the environment in the parsing context, but does not actually create an AST node when there is no initializer. Because it doesn't create an AST node, the helper function that calls this parsing method may emit an EmptyExpression AST node, or may emit a CommaNode where there aren't nodes in the CommaNode representing empty initializers. We should emit an AST node here that represents declaring variables without initializers because it gives high fidelity type profiling the opportunity to emit code that observes this variable.
Created attachment 235940 [details] work in progress What do you guys think of this patch?
Comment on attachment 235940 [details] work in progress View in context: https://bugs.webkit.org/attachment.cgi?id=235940&action=review I think this approach is good. Some edits are in order. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:1789 > + generator.emitDebugHook(WillExecuteStatement, lineNo(), startOffset(), lineStartOffset()); This seems like it might be too much debug hook. Now, we'll have two debug hooks for the statement "var x;": one for the VarStatementNode, and one for the EmptyVarExpressionNode. Probably the right thing to do is not to debug hook here, since usually only statements debug hook, and we don't stop the debugger in the middle of expressions. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:1791 > + if (generator.isProfilingTypesWithHighFidelity()) { It's nice to make this a "not" check followed by early return. That way, the rest of the code doesn't have to indent, and you rarely get super-indented code. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:1796 > + //RegisterID* finalDest = generator.finalDestination(dst); Nix this. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:1804 > + // It's safe to return null here because this node will always be part of VarStatementNode which ignores our return value. "part of" => "a child node of" > Source/JavaScriptCore/parser/ASTBuilder.h:649 > - > + Revert whitespace change please.
Created attachment 236691 [details] patch Patch with Geoff's suggestions.
Comment on attachment 236691 [details] patch r=me
Comment on attachment 236691 [details] patch Clearing flags on attachment: 236691 Committed r172717: <http://trac.webkit.org/changeset/172717>
All reviewed patches have been landed. Closing bug.