Bug 139230 - The parser should allocate all pieces of the AST
Summary: The parser should allocate all pieces of the AST
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-03 11:36 PST by Geoffrey Garen
Modified: 2014-12-03 13:54 PST (History)
3 users (show)

See Also:


Attachments
Patch (23.67 KB, patch)
2014-12-03 11:47 PST, Geoffrey Garen
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2014-12-03 11:36:29 PST
The parser should allocate all pieces of the AST
Comment 1 Geoffrey Garen 2014-12-03 11:47:57 PST
Created attachment 242509 [details]
Patch
Comment 2 Oliver Hunt 2014-12-03 12:04:42 PST
Comment on attachment 242509 [details]
Patch

r=me.  There's so much cruft left over from the old bison parser :(
Comment 3 Mark Lam 2014-12-03 12:19:30 PST
Comment on attachment 242509 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=242509&action=review

> Source/JavaScriptCore/parser/ASTBuilder.h:322
> +    PropertyNode* createProperty(VM*, double propertyName, ExpressionNode* node, PropertyNode::Type type, bool)

is there a reason this unused VM* arg can't be removed?
Comment 4 Geoffrey Garen 2014-12-03 12:29:25 PST
> > Source/JavaScriptCore/parser/ASTBuilder.h:322
> > +    PropertyNode* createProperty(VM*, double propertyName, ExpressionNode* node, PropertyNode::Type type, bool)
> 
> is there a reason this unused VM* arg can't be removed?

Though ASTBuilder uses its m_vm data member instead of the VM* argument, SyntaxChecker does not have an m_vm data member, and does use the VM* argument.

This is an oddity that could use cleaning up, but I didn't clean it up in this patch.
Comment 5 Geoffrey Garen 2014-12-03 13:54:22 PST
Committed r176754: <http://trac.webkit.org/changeset/176754>