Parse arrow functions at proper precedence level

Parse arrow functions at proper precedence level

BUG=v8:4211
LOG=Y
R=rossberg@chromium.org

Committed: https://crrev.com/9271b0ccf9ddb217deb1f0b9ef9b59b64dc40214
Cr-Commit-Position: refs/heads/master@{#30298}

[wingo, 2015-08-20 13:07:38]

Patchset #1 (id:1) has been deleted

[wingo, 2015-08-20 13:09:17]

+mstarzinger for the TF bits.  WDYT Andreas?  This patch is to fix a precedence
corner case that is irritating but IMO not worth uplifting to stable because of
the invasive aspects.  Perhaps it should be applied to dev though so as not to
inadvertently break the web; dunno.  Perhaps not.

[wingo, 2015-08-20 13:37:46]

The CQ bit was checked by wingo@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2015-08-20 13:38:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1286383005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1286383005/20001

[Michael Starzinger, 2015-08-20 13:38:10]

mstarzinger@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[Michael Starzinger, 2015-08-20 13:38:10]

The compiler and full-code parts look OK, didn't look at the rest (or at the
general issue this CL is about).

[Michael Starzinger, 2015-08-20 13:38:21]

mstarzinger@chromium.org changed reviewers:
- mstarzinger@chromium.org

[commit-bot: I haz the power, 2015-08-20 14:38:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-20 14:38:20]

Dry run: This issue passed the CQ dry run.

[rossberg, 2015-08-20 16:50:00]

https://codereview.chromium.org/1286383005/diff/20001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1286383005/diff/20001/src/preparser.h#newcode...
src/preparser.h:2288: result =
Nit: unnecessary line break?

https://codereview.chromium.org/1286383005/diff/20001/src/preparser.h#newcode...
src/preparser.h:2290: // Patterns not allowed as rest parameters.  There is no
way we can
Nit: insert "are".

FWIW, Microsoft just proposed to allow patterns there in ES7. But no reason to
worry about that now...

https://codereview.chromium.org/1286383005/diff/20001/src/preparser.h#newcode...
src/preparser.h:2871: // ValidateExpression(&arrow_formals_classifier,
CHECK_OK);
Hm?

[wingo, 2015-08-21 09:50:01]

Patchset #2 (id:40001) has been deleted

[wingo, 2015-08-21 09:50:49]

Fixed nits; PTAL.  Thanks :)

https://codereview.chromium.org/1286383005/diff/20001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1286383005/diff/20001/src/preparser.h#newcode...
src/preparser.h:2288: result =
On 2015/08/20 16:50:00, rossberg wrote:
> Nit: unnecessary line break?

Done.

https://codereview.chromium.org/1286383005/diff/20001/src/preparser.h#newcode...
src/preparser.h:2290: // Patterns not allowed as rest parameters.  There is no
way we can
On 2015/08/20 16:50:00, rossberg wrote:
> Nit: insert "are".
> 
> FWIW, Microsoft just proposed to allow patterns there in ES7. But no reason to
> worry about that now...

Done.

https://codereview.chromium.org/1286383005/diff/20001/src/preparser.h#newcode...
src/preparser.h:2871: // ValidateExpression(&arrow_formals_classifier,
CHECK_OK);
On 2015/08/20 16:50:00, rossberg wrote:
> Hm?

A vestige of a previous patch, oops!  Removed.

[rossberg, 2015-08-21 10:33:19]

lgtm

[wingo, 2015-08-21 10:34:23]

The CQ bit was checked by wingo@igalia.com

[commit-bot: I haz the power, 2015-08-21 10:34:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1286383005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1286383005/60001

[Michael Starzinger, 2015-08-21 10:38:11]

mstarzinger@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[Michael Starzinger, 2015-08-21 10:38:11]

LGTM on compiler directory.

[commit-bot: I haz the power, 2015-08-21 11:33:33]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2015-08-21 11:33:54]

Patchset 2 (id:??) landed as
https://crrev.com/9271b0ccf9ddb217deb1f0b9ef9b59b64dc40214
Cr-Commit-Position: refs/heads/master@{#30298}

[fennyfanny655, 2015-08-21 12:01:53]

fennyfanny655@gmail.com changed reviewers:
+ fennyfanny655@gmail.com

[Michael Achenbach, 2015-08-21 17:07:35]

machenbach@chromium.org changed reviewers:
+ machenbach@chromium.org

[Michael Achenbach, 2015-08-21 17:07:36]

Guess this breaks layout test expectations:
http://build.chromium.org/p/client.v8.fyi/console?branch=master

Please revert or adapt blink-upstream asap.

[Yang, 2015-08-24 06:56:44]

A revert of this CL (patchset #2 id:60001) has been created in
https://codereview.chromium.org/1315503002/ by yangguo@chromium.org.

The reason for reverting is: Breaks layout test. Please change test expectation
on blink first.

---
/mnt/data/b/build/slave/V8-Blink_Linux_64/build/layout-test-results/inspector/sources/debugger-pause/debugger-pause-in-internal-expected.txt
+++
/mnt/data/b/build/slave/V8-Blink_Linux_64/build/layout-test-results/inspector/sources/debugger-pause/debugger-pause-in-internal-actual.txt
@@ -1,4 +1,4 @@
-CONSOLE ERROR: line 9: Uncaught SyntaxError: Expected () to start arrow
function, but got '}' instead of '=>'
+CONSOLE ERROR: line 9: Uncaught SyntaxError: Unexpected token )
 Tests that pause on exception in internal script does not crash.
 
 Script source was shown..