[es6] Parameter scopes for sloppy eval

[es6] Parameter scopes for sloppy eval

This CL is a nightmare! For the utterly irrelevant edge case of a sloppy function with non-simple parameters and a call to direct eval, like here,

  let x = 1;
  function f(g = () => x) {
    var y
    eval("var x = 2")
    return g() + x  // f() = 3
  }

we have to do all of the following, on top of the declaration block ("varblock") contexts we already introduce around the body:

- Introduce the ability for varblock contexts to have both a ScopeInfo and an extension object (e.g., the body varblock in the example will contain both a static var y and a dynamic var x). No other scope needs that. Since there are no context slots left, a special new struct is introduced that pairs up scope info and extension object.

- When declaring lookup slots in the runtime, this new struct is allocated in the case where an extension object has to be added to a block scope (at which point the block's extension slot still contains a plain ScopeInfo).

- While at it, introduce some abstraction to access context extension slots in a more controlled manner, in order to keep special-casing to a minimum.

- Make sure that even empty varblock contexts do not get optimised away when they contain a sloppy eval, so that they can host the potential extension object.

- Extend dynamic search for declaration contexts (used by sloppy direct eval) to recognize varblock contexts.

- In the parser, if a function has a sloppy direct eval, introduce an additional varblock scope around each non-simple (desugared) parameter, as required by the spec to contain possible dynamic var bindings.

- In the pattern rewriter, add the ability to hoist the named variables the pattern declares to an outer scope. That is required because the actual destructuring has to be evaluated inside the protecting varblock scope, but the bindings that the desugaring introduces are in the outer scope.

- ScopeInfos need to save the information whether a block is a varblock, to make sloppy eval calls work correctly that deserialise them as part of the scope chain.

- Add the ability to materialize block scopes with extension objects in the debugger. Likewise, enable setting extension variables in block scopes via the debugger interface.

- While at it, refactor and unify some respective code in the debugger.

Sorry, this CL is large. I could try to split it up, but everything is rather entangled.

@mstarzinger: Please review the changes to contexts.
@yangguo: Please have a look at the debugger stuff.

R=littledan@chromium.org, mstarzinger@chromium.org, yangguo@chromium.org
BUG=v8:811, v8:2160
LOG=N

Committed: https://crrev.com/365fd7bc351705e4fb181741829f89e580549daf
Cr-Commit-Position: refs/heads/master@{#30295}

[Yang, 2015-08-20 09:08:56]

On 2015/08/19 16:10:23, rossberg wrote:

Very confused. In the example, is the 'x' in the default parameter arrow
function supposed to bind to the 'x' introduced later in eval? Wtf.

[Yang, 2015-08-20 09:09:22]

On 2015/08/20 09:08:56, Yang wrote:
> On 2015/08/19 16:10:23, rossberg wrote:
> 
> Very confused. In the example, is the 'x' in the default parameter arrow
> function supposed to bind to the 'x' introduced later in eval? Wtf.

Wait nvm.

[Yang, 2015-08-20 09:17:08]

On 2015/08/20 09:09:22, Yang wrote:
> On 2015/08/20 09:08:56, Yang wrote:
> > On 2015/08/19 16:10:23, rossberg wrote:
> > 
> > Very confused. In the example, is the 'x' in the default parameter arrow
> > function supposed to bind to the 'x' introduced later in eval? Wtf.
> 
> Wait nvm.

debugger part lgtm.

[Michael Starzinger, 2015-08-20 10:30:00]

LGTM on compiler, full-codegen, objects and contexts. Didn't look at the rest.

https://codereview.chromium.org/1292753007/diff/1/src/contexts.cc
File src/contexts.cc (right):

https://codereview.chromium.org/1292753007/diff/1/src/contexts.cc#newcode65
src/contexts.cc:65: return ext->IsSloppyBlockWithEvalContextExtension()
nit: As discussed offline, it is not immediately clear from the code that a
block-context containing the new pair will always be a declaration scope (i.e.
that pair->scope_info()->is_declaration_scope() will always hold). Can we leave
a comment or DCHECK in that regard here?

[Dan Ehrenberg, 2015-08-20 21:24:48]

lgtm

Such a weird spec, but your implementation makes sense. I can't think of a
simpler way. Why don't the default parameter direct evals just run within the
function's existing var scope? (Or, why 1JS?!)

[rossberg, 2015-08-21 09:53:37]

On 2015/08/20 21:24:48, Dan Ehrenberg wrote:
> Such a weird spec, but your implementation makes sense. I can't think of a
> simpler way. Why don't the default parameter direct evals just run within the
> function's existing var scope? (Or, why 1JS?!)

Because the var scope is not in scope for parameters. Otherwise, e.g. this

  function f() { return 1 }
  function g(x = f()) {
    // ...way down
    var f = 2
  }

would have a rather surprising result (g() === undefined). I'd actually be fine
if this happened with var (just don't use it), but for legacy reasons, inner
functions are var bindings, too, unless inside a block.

[rossberg, 2015-08-21 10:00:40]

https://codereview.chromium.org/1292753007/diff/1/src/contexts.cc
File src/contexts.cc (right):

https://codereview.chromium.org/1292753007/diff/1/src/contexts.cc#newcode65
src/contexts.cc:65: return ext->IsSloppyBlockWithEvalContextExtension()
On 2015/08/20 10:30:00, Michael Starzinger wrote:
> nit: As discussed offline, it is not immediately clear from the code that a
> block-context containing the new pair will always be a declaration scope (i.e.
> that pair->scope_info()->is_declaration_scope() will always hold). Can we
leave
> a comment or DCHECK in that regard here?

Done: added a comment here and a DCHECK to the factory function for
SloppyBlockWithBla itself.

[rossberg, 2015-08-21 10:00:46]

The CQ bit was checked by rossberg@chromium.org

[rossberg, 2015-08-21 10:00:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
littledan@chromium.org, mstarzinger@chromium.org, yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/1292753007/#ps40001
(title: "Comments")

[commit-bot: I haz the power, 2015-08-21 10:00:53]

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

[commit-bot: I haz the power, 2015-08-21 10:58:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-08-21 10:58:54]

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

[ruv, 2015-09-02 12:22:32]

On 2015/08/19 16:10:23, rossberg wrote:
> - In the parser, if a function has a sloppy direct eval, introduce an
additional varblock scope around each non-simple (desugared) parameter, as
required by the spec to contain possible dynamic var bindings. 

Could you please clarify, does the engine take into account that in strict mode
such eval doesn't introduce new variables into the surrounding scope?   So, in
such case no need to allow possible dynamic var bindings.

[rossberg, 2015-09-02 14:43:43]

On 2015/09/02 12:22:32, ruv wrote:
> On 2015/08/19 16:10:23, rossberg wrote:
> > - In the parser, if a function has a sloppy direct eval, introduce an
> additional varblock scope around each non-simple (desugared) parameter, as
> required by the spec to contain possible dynamic var bindings. 
> 
> Could you please clarify, does the engine take into account that in strict
mode
> such eval doesn't introduce new variables into the surrounding scope?   So, in
> such case no need to allow possible dynamic var bindings.

Yes, strict cannot contain a "sloppy direct eval". (Though do-expressions may be
another source of vars in the future, even in strict mode.)