[es6] Fix scoping for default parameters in arrow functions

[es6] Fix scoping for default parameters in arrow functions

When eagerly parsing arrow functions, expressions in default
parameter initializers are parsed in the enclosing scope,
rather than in the function's scope (since that scope does not
yet exist). This leads to VariableProxies being added to the
wrong scope, and scope chains for FunctionLiterals being incorrect.

This patch addresses these problems by adding a subclass of
AstExpressionVisitor that moves VariableProxies to the proper
scope and fixes up scope chains of FunctionLiterals.

This is a revert of the revert https://crrev.com/e41614a058426fb6102e4ab2dd4f98997f00c0fc
with a much-improved (though not yet perfect) Scope::ResetOuterScope
method which properly fixes not only the outer_scope_ pointer but also
fixes the inner_scope_ list in the relevant outer_scopes.

More work likely still needs to be done to make this work completely,
but it's very close to correct.

BUG=v8:4395
LOG=y

Committed: https://crrev.com/02e4d21f4ce799c7d4e3500fa2cf96fc447456ff
Cr-Commit-Position: refs/heads/master@{#31435}

[adamk, 2015-10-20 21:25:15]

Description was changed from

==========
Revert "Revert of [es6] Fix scoping for default parameters in arrow functions
(patchset #5 id:80001 of https://codereview.chromium.org/1405313002/ )"

This reverts commit e41614a058426fb6102e4ab2dd4f98997f00c0fc.

BUG=
==========

to

==========
[es6] Fix scoping for default parameters in arrow functions

When eagerly parsing arrow functions, expressions in default
parameter initializers are parsed in the enclosing scope,
rather than in the function's scope (since that scope does not
yet exist). This leads to VariableProxies being added to the
wrong scope, and scope chains for FunctionLiterals being incorrect.

This patch addresses these problems by adding a subclass of
AstExpressionVisitor that moves VariableProxies to the proper
scope and fixes up scope chains of FunctionLiterals.

More work likely still needs to be done to make this work completely,
but it's very close to correct.

BUG=v8:4395
LOG=y
==========

[adamk, 2015-10-20 21:27:40]

Description was changed from

==========
[es6] Fix scoping for default parameters in arrow functions

When eagerly parsing arrow functions, expressions in default
parameter initializers are parsed in the enclosing scope,
rather than in the function's scope (since that scope does not
yet exist). This leads to VariableProxies being added to the
wrong scope, and scope chains for FunctionLiterals being incorrect.

This patch addresses these problems by adding a subclass of
AstExpressionVisitor that moves VariableProxies to the proper
scope and fixes up scope chains of FunctionLiterals.

More work likely still needs to be done to make this work completely,
but it's very close to correct.

BUG=v8:4395
LOG=y
==========

to

==========
[es6] Fix scoping for default parameters in arrow functions

When eagerly parsing arrow functions, expressions in default
parameter initializers are parsed in the enclosing scope,
rather than in the function's scope (since that scope does not
yet exist). This leads to VariableProxies being added to the
wrong scope, and scope chains for FunctionLiterals being incorrect.

This patch addresses these problems by adding a subclass of
AstExpressionVisitor that moves VariableProxies to the proper
scope and fixes up scope chains of FunctionLiterals.

This is a revert of the revert
https://crrev.com/e41614a058426fb6102e4ab2dd4f98997f00c0fc
with a much-improved (though not yet perfect) Scope::ResetOuterScope
method which properly fixes not only the outer_scope_ pointer but also
fixes the inner_scope_ list in the relevant outer_scopes.

More work likely still needs to be done to make this work completely,
but it's very close to correct.

BUG=v8:4395
LOG=y
==========

[adamk, 2015-10-20 21:28:36]

adamk@chromium.org changed reviewers:
+ rossberg@chromium.org

[adamk, 2015-10-20 21:28:36]

Patch set 2 includes the fix for the failures on the waterfall.

[rossberg, 2015-10-21 10:46:59]

lgtm

https://codereview.chromium.org/1414283002/diff/20001/src/scopes.h
File src/scopes.h (right):

https://codereview.chromium.org/1414283002/diff/20001/src/scopes.h#newcode118
src/scopes.h:118: void ResetOuterScope(Scope* outer_scope);
Nit: "Reset" is not very telling, how about "Replace"?

[adamk, 2015-10-21 10:58:29]

The CQ bit was checked by adamk@chromium.org

[adamk, 2015-10-21 10:58: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/1414283002/#ps40001
(title: "Reset -> replace")

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

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

[adamk, 2015-10-21 10:58:35]

https://codereview.chromium.org/1414283002/diff/20001/src/scopes.h
File src/scopes.h (right):

https://codereview.chromium.org/1414283002/diff/20001/src/scopes.h#newcode118
src/scopes.h:118: void ResetOuterScope(Scope* outer_scope);
On 2015/10/21 10:46:59, rossberg wrote:
> Nit: "Reset" is not very telling, how about "Replace"?

Done.

[commit-bot: I haz the power, 2015-10-21 10:59:35]

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

[commit-bot: I haz the power, 2015-10-21 10:59:35]

Try jobs failed on following builders:
  v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/1...)
  v8_linux_arm_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel/builds/9207)
  v8_linux_dbg on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg/builds/9822)
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel/builds/10782)
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/10925)

[adamk, 2015-10-21 11:09:00]

The CQ bit was checked by adamk@chromium.org

[adamk, 2015-10-21 11:09:00]

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/1414283002/#ps60001
(title: "Rebased")

[commit-bot: I haz the power, 2015-10-21 11:09:07]

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

[commit-bot: I haz the power, 2015-10-21 12:04:18]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-21 12:04:31]

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