Closed Bug 1216966 Opened 4 years ago Closed 4 years ago

Miscellaneous js/src/frontend tidying

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: jorendorff, Assigned: jorendorff)

References

Details

Attachments

(5 files)

No description provided.
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
Attachment #8676865 - Attachment is obsolete: true
Attachment #8676865 - Flags: review?(efaustbmo)
Attachment #8676865 - Attachment is obsolete: false
Attachment #8676865 - Flags: review?(efaustbmo)
(Sorry for that part 2, I made the change while reading and trying to understand code, and I still think it's a little better so I kept it.)
Blocks: 1217001
Comment on attachment 8676865 [details] [diff] [review]
Part 1: Splurge and use separate error messages for generator comprehensions that need more parentheses and yield expressions that need more parentheses

Review of attachment 8676865 [details] [diff] [review]:
-----------------------------------------------------------------

Such opulence. Such...conspicuous consumption.
Attachment #8676865 - Flags: review?(efaustbmo) → review+
Comment on attachment 8676867 [details] [diff] [review]
Part 2: Pointlessly tweak a comment about comprehension syntax

Review of attachment 8676867 [details] [diff] [review]:
-----------------------------------------------------------------

It's clearer this way. There's value, here.
Attachment #8676867 - Flags: review?(efaustbmo) → review+
Comment on attachment 8676869 [details] [diff] [review]
Part 3: Remove redundant method Parser::parenExprOrGeneratorComprehension

Review of attachment 8676869 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.

::: js/src/frontend/Parser.cpp
@@ +9386,5 @@
> +
> +            // Now just return something that will allow parsing to continue.
> +            // It doesn't matter what; when we reach the =>, we will rewind and
> +            // reparse the whole arrow function. See Parser::assignExpr.
> +            return handler.newNullLiteral(pos());

Aside: Man, what a hack! I hadn't read this comment before.
Attachment #8676869 - Flags: review?(efaustbmo) → review+
Attachment #8676870 - Flags: review?(efaustbmo) → review+
Comment on attachment 8676873 [details] [diff] [review]
Part 5: Code organization and comments around parsing comprehensions

Review of attachment 8676873 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/Parser.cpp
@@ +7572,5 @@
> + * TC39 decided to drop it.)
> + *
> + * Legacy generator expressions evaluate to legacy generators (using the
> + * StopIteration protocol); ES6-era generator expressions evaluate to ES6
> + * generators (using the `{done:, value:}` protocol).

This is a great comment. It's quite clear, and tells the history of the situation concisely.

@@ +8467,5 @@
>  }
>  
> +
> +/* * */
> +

Sssure. I think this comment is maybe unnecessary, but it does bracket the newly added Comprehensions comment, so it's fine.
Attachment #8676873 - Flags: review?(efaustbmo) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f659dd470d83d23024bf1bcc173614cb8a12643
Bug 1216966 - Part 1: Splurge and use separate error messages for generator comprehensions that need more parentheses and yield expressions that need more parentheses. r=efaust.

https://hg.mozilla.org/integration/mozilla-inbound/rev/30f2d69ecd7c3d9e8faa167f49adf9bd1c615fcc
Bug 1216966 - Part 2: Tweak a comment about comprehension syntax. r=efaust.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1d4b1adfb5724a94ae6c82c7c211b607ee1e9a4a
Bug 1216966 - Part 3: Remove redundant method Parser::parenExprOrGeneratorComprehension. r=efaust.

https://hg.mozilla.org/integration/mozilla-inbound/rev/cd16e0e4852bbced3db97c67b849653dab26d123
Bug 1216966 - Part 4: Update a big comment about for-loops in Parser.cpp. r=efaust.

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb5b95d5f621a62758a4d26f06ee15d98f2ac43f
Bug 1216966 - Part 5: Code organization and comments around parsing comprehensions. r=efaust.
You need to log in before you can comment on or make changes to this bug.