[parser] make kInvalidLhsInFor a SyntaxError

[parser] make kInvalidLhsInFor a SyntaxError

Second item in section 13.7.5.1 states that the error should be a
SyntaxError, when previously CheckAndRewriteReferenceExpression
would always emit a ReferenceError.

BUG=v8:4373
R=adamk, rossberg
LOG=N
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel

Committed: https://crrev.com/9b15445e6d4a3cb9b1655b9e57ac3e147118195a
Cr-Commit-Position: refs/heads/master@{#30184}

[caitp (gmail), 2015-08-14 17:06:17]

Hi, PTAL, I wanted to take on a smaller bug since it's been a few weeks.

Going to add a 2nd patch shortly to address the self-review I've done

https://codereview.chromium.org/1292393002/diff/1/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1292393002/diff/1/src/preparser.h#newcode3982
src/preparser.h:3982: : this->NewThrowSyntaxError(message, pos);
I'm not sure if this distinction is necessary or good here --- getting rid of
this does make the patch a lot smaller.

SpiderMonkey does not make the distinction, at any rate.

[adamk, 2015-08-15 01:06:15]

lgtm

https://codereview.chromium.org/1292393002/diff/1/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1292393002/diff/1/src/preparser.h#newcode3982
src/preparser.h:3982: : this->NewThrowSyntaxError(message, pos);
On 2015/08/14 17:06:17, caitp wrote:
> I'm not sure if this distinction is necessary or good here --- getting rid of
> this does make the patch a lot smaller.
> 
> SpiderMonkey does not make the distinction, at any rate.

Seems fine to me, and also makes sense tonot throw a SyntaxError at runtime...

[caitp (gmail), 2015-08-15 02:09:44]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2015-08-15 02:09:59]

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

[caitp (gmail), 2015-08-15 02:10:06]

On 2015/08/15 01:06:15, adamk wrote:
> lgtm
> 
> https://codereview.chromium.org/1292393002/diff/1/src/preparser.h
> File src/preparser.h (right):
> 
> https://codereview.chromium.org/1292393002/diff/1/src/preparser.h#newcode3982
> src/preparser.h:3982: : this->NewThrowSyntaxError(message, pos);
> On 2015/08/14 17:06:17, caitp wrote:
> > I'm not sure if this distinction is necessary or good here --- getting rid
of
> > this does make the patch a lot smaller.
> > 
> > SpiderMonkey does not make the distinction, at any rate.
> 
> Seems fine to me, and also makes sense tonot throw a SyntaxError at runtime...

Thanks for the look

[commit-bot: I haz the power, 2015-08-15 02:23:13]

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

[commit-bot: I haz the power, 2015-08-15 02:23:14]

Try jobs failed on following builders:
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/8594)

[caitp (gmail), 2015-08-15 02:48:35]

On 2015/08/15 02:23:14, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/8594)

The weird thing with those webkit test failures is that, a lot of the failures
behave exactly the same way in Canary. Looking into it

[caitp (gmail), 2015-08-15 03:02:55]

On 2015/08/15 02:48:35, caitp wrote:
> On 2015/08/15 02:23:14, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/8594)
> 
> The weird thing with those webkit test failures is that, a lot of the failures
> behave exactly the same way in Canary. Looking into it

the test helper functions seem to be doing the wrong thing, might make sense to
kill this test and just rely on v8's syntax tests.
If people agree, will send another cl to remove it, otherwise I'll try to make
it usable

[caitp (gmail), 2015-08-15 08:41:29]

On 2015/08/15 03:02:55, caitp wrote:
> On 2015/08/15 02:48:35, caitp wrote:
> > On 2015/08/15 02:23:14, commit-bot: I haz the power wrote:
> > > Try jobs failed on following builders:
> > >   v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
> > >
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/8594)
> > 
> > The weird thing with those webkit test failures is that, a lot of the
failures
> > behave exactly the same way in Canary. Looking into it
> 
> the test helper functions seem to be doing the wrong thing, might make sense
to
> kill this test and just rely on v8's syntax tests.
> If people agree, will send another cl to remove it, otherwise I'll try to make
> it usable

eh, think I was wrong before --- but not totally sure this is correct now.

[caitp (gmail), 2015-08-15 17:03:20]

On 2015/08/15 08:41:29, caitp wrote:
> On 2015/08/15 03:02:55, caitp wrote:
> > On 2015/08/15 02:48:35, caitp wrote:
> > > On 2015/08/15 02:23:14, commit-bot: I haz the power wrote:
> > > > Try jobs failed on following builders:
> > > >   v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
> > > >
> > >
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/8594)
> > > 
> > > The weird thing with those webkit test failures is that, a lot of the
> failures
> > > behave exactly the same way in Canary. Looking into it
> > 
> > the test helper functions seem to be doing the wrong thing, might make sense
> to
> > kill this test and just rely on v8's syntax tests.
> > If people agree, will send another cl to remove it, otherwise I'll try to
make
> > it usable
> 
> eh, think I was wrong before --- but not totally sure this is correct now.

As far as I can see, these are correct, after a bit more looking.

[caitp (gmail), 2015-08-15 17:08:12]

The CQ bit was checked by caitpotter88@gmail.com

[caitp (gmail), 2015-08-15 17:08:12]

The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/1292393002/#ps60001
(title: "Correct tests")

[commit-bot: I haz the power, 2015-08-15 17:08:19]

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

[commit-bot: I haz the power, 2015-08-15 17:57:08]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-08-15 17:57:28]

Patchset 4 (id:??) landed as
https://crrev.com/9b15445e6d4a3cb9b1655b9e57ac3e147118195a
Cr-Commit-Position: refs/heads/master@{#30184}