Bug 155384 - assignments in for-in/for-of header not allowed
Summary: assignments in for-in/for-of header not allowed
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
Depends on:
Reported: 2016-03-11 15:14 PST by Saam Barati
Modified: 2016-03-14 11:38 PDT (History)
10 users (show)

See Also:

patch (13.43 KB, patch)
2016-03-11 16:49 PST, Saam Barati
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2016-03-11 15:14:07 PST
Comment 1 Saam Barati 2016-03-11 16:49:33 PST
Created attachment 273785 [details]
Comment 2 Darin Adler 2016-03-13 14:41:39 PDT
Comment on attachment 273785 [details]

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

> Source/JavaScriptCore/parser/ASTBuilder.h:185
> +    bool isNewTarget(ExpressionNode* expr) { return expr->isNewTarget(); }

I suggest either the word "expression" or the word "node" rather than the abbreviation "expr".

> Source/JavaScriptCore/parser/Nodes.h:562
> +        bool isNewTarget() const override { return true; }

We’ve been choosing to “final” rather than “override” in cases like this one in our virtual function style discussions for the WebKit project. Also, should just make it private since there’s no point calling it if you already have a reference or pointer to a NewTargetNode.
Comment 3 Saam Barati 2016-03-14 11:38:55 PDT
Thanks for the review. I made the suggestions you recommended.
landed in: