Rescope arrow-function parameter lists by moving the delta to the parameter scope

Rescope arrow-function parameter lists by moving the delta to the parameter scope

This replaces the AstVisitor approach for scope rewriting with a Scope-only
solution, using a new Scope::Snapshot object that keeps track of inner scopes,
unresolved variables, and temps.

The only use of the AstVisitor is now for parameter varblock scopes introduced
due to sloppy eval in parameters, which greatly simplifies the rewriter
as it no longer needs to handle temps. A future CL may be able to
eliminate it altogether by taking a snapshot per function argument.

Based on verwaest's https://codereview.chromium.org/2166023002/.

BUG=v8:5226
Committed: https://crrev.com/e9dea58fa22fe57d5b256e3ca4310822f2d3c950
Cr-Commit-Position: refs/heads/master@{#37989}

[adamk, 2016-07-21 19:57:56]

adamk@chromium.org changed reviewers:
+ nikolaos@chromium.org, verwaest@chromium.org

[adamk, 2016-07-21 19:58:31]

The CQ bit was checked by adamk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 19:58:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[adamk, 2016-07-21 20:00:24]

The CQ bit was checked by adamk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 20:00:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-21 20:24:35]

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

[commit-bot: I haz the power, 2016-07-21 20:24:36]

Dry run: This issue passed the CQ dry run.

[Toon Verwaest, 2016-07-22 08:28:06]

lgtm, thanks for taking over!

[nickie, 2016-07-22 09:05:31]

LGTM, one comment.

https://codereview.chromium.org/2171703004/diff/20001/src/ast/scopes.cc
File src/ast/scopes.cc (right):

https://codereview.chromium.org/2171703004/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:390: DCHECK_EQ(new_parent, outer_scope_->inner_scope_);
I'm not sure why we check this again.  It's checked in line 371.  Do we fear
that outer_scope_->inner_scope_ will have changed since then?  That could only
have happened if new_parent_ == outer_scope_, which is rather impossible, but if
we're afraid it may happen, I suggest checking that directly.

[adamk, 2016-07-22 23:04:05]

The CQ bit was checked by adamk@chromium.org

[adamk, 2016-07-22 23:04:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
nikolaos@chromium.org, verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/2171703004/#ps40001
(title: "Remove unnecessary DCHECK")

[commit-bot: I haz the power, 2016-07-22 23:04:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[adamk, 2016-07-22 23:04:34]

https://codereview.chromium.org/2171703004/diff/20001/src/ast/scopes.cc
File src/ast/scopes.cc (right):

https://codereview.chromium.org/2171703004/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:390: DCHECK_EQ(new_parent, outer_scope_->inner_scope_);
On 2016/07/22 09:05:31, nickie wrote:
> I'm not sure why we check this again.  It's checked in line 371.  Do we fear
> that outer_scope_->inner_scope_ will have changed since then?  That could only
> have happened if new_parent_ == outer_scope_, which is rather impossible, but
if
> we're afraid it may happen, I suggest checking that directly.

I agree that there's no need to check this again, removed.

[commit-bot: I haz the power, 2016-07-22 23:28:22]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-22 23:31:03]

Description was changed from

==========
Rescope arrow-function parameter lists by moving the delta to the parameter
scope

This replaces the AstVisitor approach for scope rewriting with a Scope-only
solution, using a new Scope::Snapshot object that keeps track of inner scopes,
unresolved variables, and temps.

The only use of the AstVisitor is now for parameter varblock scopes introduced
due to sloppy eval in parameters, which greatly simplifies the rewriter
as it no longer needs to handle temps. A future CL may be able to
eliminate it altogether by taking a snapshot per function argument.

Based on verwaest's https://codereview.chromium.org/2166023002/.

BUG=v8:5226
==========

to

==========
Rescope arrow-function parameter lists by moving the delta to the parameter
scope

This replaces the AstVisitor approach for scope rewriting with a Scope-only
solution, using a new Scope::Snapshot object that keeps track of inner scopes,
unresolved variables, and temps.

The only use of the AstVisitor is now for parameter varblock scopes introduced
due to sloppy eval in parameters, which greatly simplifies the rewriter
as it no longer needs to handle temps. A future CL may be able to
eliminate it altogether by taking a snapshot per function argument.

Based on verwaest's https://codereview.chromium.org/2166023002/.

BUG=v8:5226

Committed: https://crrev.com/e9dea58fa22fe57d5b256e3ca4310822f2d3c950
Cr-Commit-Position: refs/heads/master@{#37989}
==========

[commit-bot: I haz the power, 2016-07-22 23:31:04]

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