[es6] parse arrow ConciseBody with accept_IN flag

[es6] parse arrow ConciseBody with accept_IN flag

Fixes corner case where arrow function ConciseBody expression does not
accept 'in' in productions.

BUG=v8:4472
LOG=N
R=wingo@igalia.com, adamk@chromium.org, rossberg@chromium.org

Committed: https://crrev.com/6b4d7f81a9faf2b0c4a4ce6e9c671ebbe5c62f79
Cr-Commit-Position: refs/heads/master@{#31271}

[caitp (gmail), 2015-10-13 21:50:00]

PTAL

I'm not a huge fan of this, because it adds an extra bit to both
SharedFunctionInfo::compiler_hints and FunctionLiteral's bitfield.

I'd appreciate suggestions on achieving the functionality without growing these
things even more

[adamk, 2015-10-14 12:46:44]

It certainly seems odd to have to store this bit of information just to make
sure that we get the right error later on. I vaguely recall there being some way
to store parse errors for functions that have early errors in them...am I
misremembering?

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

https://codereview.chromium.org/1407633002/diff/1/src/preparser.h#newcode3905
src/preparser.h:3905: bool concise = false;
I don't see this used anywhere (other than it being set to 'true' below), can it
be removed?

[caitp (gmail), 2015-10-14 13:20:18]

On 2015/10/14 12:46:44, adamk wrote:
> It certainly seems odd to have to store this bit of information just to make
> sure that we get the right error later on. I vaguely recall there being some
way
> to store parse errors for functions that have early errors in them...am I
> misremembering?
> 

Doesn't ring a bell, but I'll see. The issue with testing is that AFAIK
cctest/test-parsing runs the full parser and preparser seperately, so the full
parser is probably missing any info about a failed preparse attempt.

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

https://codereview.chromium.org/1407633002/diff/1/src/preparser.h#newcode3905
src/preparser.h:3905: bool concise = false;
On 2015/10/14 12:46:44, adamk wrote:
> I don't see this used anywhere (other than it being set to 'true' below), can
it
> be removed?

probably left over from an older version of the patch, it can go

[adamk, 2015-10-14 13:31:16]

Isn't the extra data needed not for "lazy parsing" but rather "lazy
compilation"? Presumably this patch causes an early error even in the preparser.

[adamk, 2015-10-14 13:37:19]

https://codereview.chromium.org/1407633002/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1407633002/diff/1/src/parser.cc#newcode1221
src/parser.cc:1221: shared_info->accept_IN(), formals, formals_classifier, &ok);
So I patched this in locally, and just passed true here. My theory is that we
never would have succeeded in reifying any function with this error, so the bit
would never even get set. Indeed I'm having trouble seeing what problem that
would cause...

[adamk, 2015-10-14 13:38:07]

On 2015/10/14 13:37:19, adamk wrote:
> https://codereview.chromium.org/1407633002/diff/1/src/parser.cc
> File src/parser.cc (right):
> 
> https://codereview.chromium.org/1407633002/diff/1/src/parser.cc#newcode1221
> src/parser.cc:1221: shared_info->accept_IN(), formals, formals_classifier,
&ok);
> So I patched this in locally, and just passed true here. My theory is that we
> never would have succeeded in reifying any function with this error, so the
bit
> would never even get set. Indeed I'm having trouble seeing what problem that
> would cause...

Sorry, jetlagged, of course the bit gets set sometimes. But it should never be
able to cause a parse failure in a function that has a SharedFunctionInfo.

[caitp (gmail), 2015-10-14 13:40:56]

On 2015/10/14 13:37:19, adamk wrote:
> https://codereview.chromium.org/1407633002/diff/1/src/parser.cc
> File src/parser.cc (right):
> 
> https://codereview.chromium.org/1407633002/diff/1/src/parser.cc#newcode1221
> src/parser.cc:1221: shared_info->accept_IN(), formals, formals_classifier,
&ok);
> So I patched this in locally, and just passed true here. My theory is that we
> never would have succeeded in reifying any function with this error, so the
bit
> would never even get set. Indeed I'm having trouble seeing what problem that
> would cause...

I was thinking this yesterday, but I decided that it would theoretically be
possible to come up with a different result when lazily parsing, due to the
parser finding different productions. It's a bit of a stretch, but keeping it
consistent seemed like a better idea.

It is sad that it's hard to store more information about this, though.

[caitp (gmail), 2015-10-14 14:05:12]

On 2015/10/14 13:40:56, caitp wrote:
> On 2015/10/14 13:37:19, adamk wrote:
> > https://codereview.chromium.org/1407633002/diff/1/src/parser.cc
> > File src/parser.cc (right):
> > 
> > https://codereview.chromium.org/1407633002/diff/1/src/parser.cc#newcode1221
> > src/parser.cc:1221: shared_info->accept_IN(), formals, formals_classifier,
> &ok);
> > So I patched this in locally, and just passed true here. My theory is that
we
> > never would have succeeded in reifying any function with this error, so the
> bit
> > would never even get set. Indeed I'm having trouble seeing what problem that
> > would cause...
> 
> I was thinking this yesterday, but I decided that it would theoretically be
> possible to come up with a different result when lazily parsing, due to the
> parser finding different productions. It's a bit of a stretch, but keeping it
> consistent seemed like a better idea.
> 
> It is sad that it's hard to store more information about this, though.

Updated with extra stuff removed

[adamk, 2015-10-14 15:14:09]

lgtm % stray variable that needs deleting.

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

https://codereview.chromium.org/1407633002/diff/20001/src/preparser.h#newcode...
src/preparser.h:3902: bool concise = false;
This still need to be removed.

[caitp (gmail), 2015-10-14 15:19:55]

The CQ bit was checked by caitpotter88@gmail.com

[caitp (gmail), 2015-10-14 15:19:55]

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/1407633002/#ps40001
(title: "Remove pesky stray var")

[commit-bot: I haz the power, 2015-10-14 15:20:03]

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

[commit-bot: I haz the power, 2015-10-14 17:39:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-14 17:39:59]

Patchset 3 (id:??) landed as
https://crrev.com/6b4d7f81a9faf2b0c4a4ce6e9c671ebbe5c62f79
Cr-Commit-Position: refs/heads/master@{#31271}