789239 - Unravel 30-line if condition in Parser.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

Because yeah.

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: Part 1 - Move the condition into a helper function

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: Part 2 - Rewrite the function using early returns

Comment 3

[Nicholas Nethercote [inactive]]

Bless you, my son.

Comment 4

[David Mandelin [:dmandelin]]

Comment on attachment 658985 [details] [diff] [review]
Part 2 - Rewrite the function using early returns

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

<applause>

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 658982 [details] [diff] [review]
Part 1 - Move the condition into a helper function

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

This patch seems to be a half-amalgam of the change here and the changes in bug 728079, which confused me a bit.

::: js/src/jit-test/tests/for-of/bug-728079-js17-3.js
@@ +1,3 @@
> +// Cleaned-up version of bug 728079 comment 0.
> +
> +version(170);

I assume this belonged in bug 728079's patch?

Comment 6

[Jeff Walden [:Waldo]]

Comment on attachment 658985 [details] [diff] [review]
Part 2 - Rewrite the function using early returns

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

::: js/src/frontend/Parser.cpp
@@ +2959,5 @@
>  {
> +    if (forDecl) {
> +        if (pn1->pn_count > 1)
> +            return false;
> +        if (pn1->isOp(JSOP_DEFCONST))

Probably shouldn't be in this change, just for more sequence points for bisection, but this can be isKind(PNK_CONST) now, I think.

Comment 7

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2085233469a0
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e9d543ae2ec

Comment 8

[Graeme McCutcheon [:graememcc]]

https://hg.mozilla.org/mozilla-central/rev/2085233469a0
https://hg.mozilla.org/mozilla-central/rev/2e9d543ae2ec