Closed
Bug 1216966
Opened 7 years ago
Closed 7 years ago
Miscellaneous js/src/frontend tidying
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: jorendorff, Assigned: jorendorff)
References
Details
Attachments
(5 files)
8.00 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
1.88 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
6.29 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
4.66 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
8.28 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8676865 -
Flags: review?(efaustbmo)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•7 years ago
|
||
Attachment #8676867 -
Flags: review?(efaustbmo)
Assignee | ||
Updated•7 years ago
|
Attachment #8676865 -
Attachment is obsolete: true
Attachment #8676865 -
Flags: review?(efaustbmo)
Assignee | ||
Updated•7 years ago
|
Attachment #8676865 -
Attachment is obsolete: false
Attachment #8676865 -
Flags: review?(efaustbmo)
Assignee | ||
Comment 3•7 years ago
|
||
(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.)
Assignee | ||
Comment 4•7 years ago
|
||
Attachment #8676869 -
Flags: review?(efaustbmo)
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #8676870 -
Flags: review?(efaustbmo)
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #8676873 -
Flags: review?(efaustbmo)
Comment 7•7 years ago
|
||
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 8•7 years ago
|
||
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 9•7 years ago
|
||
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+
Updated•7 years ago
|
Attachment #8676870 -
Flags: review?(efaustbmo) → review+
Comment 10•7 years ago
|
||
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+
Assignee | ||
Comment 11•7 years ago
|
||
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.
Comment 12•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6f659dd470d8 https://hg.mozilla.org/mozilla-central/rev/30f2d69ecd7c https://hg.mozilla.org/mozilla-central/rev/1d4b1adfb572 https://hg.mozilla.org/mozilla-central/rev/cd16e0e4852b https://hg.mozilla.org/mozilla-central/rev/bb5b95d5f621
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in
before you can comment on or make changes to this bug.
Description
•