Fix arrow functions requiring context without slots.

Fix arrow functions requiring context without slots.

This fixes a corner-case where arrow functions that require a context
allocate none, because there are no additional slots allocated. Note
that this didn't happen with true function scopes because they always
had at least the receiver slot.

The outcome was a context chain that no longer was in sync with the
scope chain, hence context slot loads were bogus. This is observable
using the DYNAMIC_LOCAL optimization in all compilers.

R=rossberg@chromium.org,wingo@igalia.com
TEST=mjsunit/harmony/regress/regress-4160
BUG=v8:4160
LOG=N

Committed: https://crrev.com/68beef53c36d8640b453da4169548cbbd61c91a9
Cr-Commit-Position: refs/heads/master@{#28788}

[rossberg, 2015-06-03 06:28:44]

Does this also work correctly if the (direct, sloppy) eval contains a 'var'
declaration?

  () => eval("var x = 666"), x

should return 666. Wouldn't the x still go to the wrong context, or am I
misunderstanding the CL?

[Michael Starzinger, 2015-06-03 07:54:53]

On 2015/06/03 06:28:44, rossberg wrote:
> Does this also work correctly if the (direct, sloppy) eval contains a 'var'
> declaration?
> 
>   () => eval("var x = 666"), x
> 
> should return 666. Wouldn't the x still go to the wrong context, or am I
> misunderstanding the CL?

Just to clarify, did you mean the following arrow?

  () => (eval("var x = 666"), x)

This one work correctly after my fix.

[rossberg, 2015-06-03 08:03:04]

On 2015/06/03 07:54:53, Michael Starzinger wrote:
> On 2015/06/03 06:28:44, rossberg wrote:
> > Does this also work correctly if the (direct, sloppy) eval contains a 'var'
> > declaration?
> > 
> >   () => eval("var x = 666"), x
> > 
> > should return 666. Wouldn't the x still go to the wrong context, or am I
> > misunderstanding the CL?
> 
> Just to clarify, did you mean the following arrow?
> 
>   () => (eval("var x = 666"), x)

Right.

> This one work correctly after my fix.

Then I'm obviously confused. :) Why does this work?

Can we have a test for that?

[Michael Starzinger, 2015-06-03 08:26:06]

On 2015/06/03 08:03:04, rossberg wrote:
> On 2015/06/03 07:54:53, Michael Starzinger wrote:
> > On 2015/06/03 06:28:44, rossberg wrote:
> > > Does this also work correctly if the (direct, sloppy) eval contains a
'var'
> > > declaration?
> > > 
> > >   () => eval("var x = 666"), x
> > > 
> > > should return 666. Wouldn't the x still go to the wrong context, or am I
> > > misunderstanding the CL?
> > 
> > Just to clarify, did you mean the following arrow?
> > 
> >   () => (eval("var x = 666"), x)
> 
> Right.
> 
> > This one work correctly after my fix.
> 
> Then I'm obviously confused. :) Why does this work?

Since the arrow function is marked as calling a sloppy eval, it will allocate a
context in the prologue (because the scope's heap count is set to
MIN_CONTEXT_SLOTS instead of 0). The eval will then install an extension object
on said context containing the dynamic declaration of "x". Any optimistic
binding to a local "x" surrounding the arrow will not hit, because said context
contains an extension object.

Before my fix, the arrow function wouldn't allocate a context (because the
scope's heap count of MIN_CONTEXT_SLOTS was still interpreted as "don't need a
context"). I guess (didn't verify) that the extension object would have been
placed on the wrong context.

> Can we have a test for that?

I added this to the regression test.

[rossberg, 2015-06-03 10:48:07]

On 2015/06/03 08:26:06, Michael Starzinger wrote:
> On 2015/06/03 08:03:04, rossberg wrote:
> > On 2015/06/03 07:54:53, Michael Starzinger wrote:
> > > On 2015/06/03 06:28:44, rossberg wrote:
> > > > Does this also work correctly if the (direct, sloppy) eval contains a
> 'var'
> > > > declaration?
> > > > 
> > > >   () => eval("var x = 666"), x
> > > > 
> > > > should return 666. Wouldn't the x still go to the wrong context, or am I
> > > > misunderstanding the CL?
> > > 
> > > Just to clarify, did you mean the following arrow?
> > > 
> > >   () => (eval("var x = 666"), x)
> > 
> > Right.
> > 
> > > This one work correctly after my fix.
> > 
> > Then I'm obviously confused. :) Why does this work?
> 
> Since the arrow function is marked as calling a sloppy eval, it will allocate
a
> context in the prologue (because the scope's heap count is set to
> MIN_CONTEXT_SLOTS instead of 0). The eval will then install an extension
object
> on said context containing the dynamic declaration of "x". Any optimistic
> binding to a local "x" surrounding the arrow will not hit, because said
context
> contains an extension object.
> 
> Before my fix, the arrow function wouldn't allocate a context (because the
> scope's heap count of MIN_CONTEXT_SLOTS was still interpreted as "don't need a
> context"). I guess (didn't verify) that the extension object would have been
> placed on the wrong context.
> 
> > Can we have a test for that?
> 
> I added this to the regression test.

Cool, thanks. LGTM

[Michael Starzinger, 2015-06-03 11:00:29]

The CQ bit was checked by mstarzinger@chromium.org

[Michael Starzinger, 2015-06-03 11:00:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
rossberg@chromium.org
Link to the patchset: https://codereview.chromium.org/1146063006/#ps40001
(title: "Cleanup test case.")

[commit-bot: I haz the power, 2015-06-03 11:00:38]

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

[commit-bot: I haz the power, 2015-06-03 11:32:32]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-03 11:32:38]

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

[wingo, 2015-06-08 15:13:47]

Late LGTM also.  Thank you Michael!