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

[superseded by adam's CL]

BUG=v8:5209

[Toon Verwaest, 2016-07-20 17:35:39]

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

[Toon Verwaest, 2016-07-20 17:35:39]

ptal.

I'm sure you'll have some interesting comments :)
I'm not entirely excited about the fact that new_parent is already in the
"inner_scope" list when reparenting, but I'm not sure it's cleaner to avoid
putting it there.

I presume we should be able to get rid of all other uses of
RewriteParameterInitializerScope in a similar fashion. I'd be interested in
getting hints in this regard.

[adamk, 2016-07-20 21:30:02]

This will let you remove one call to RewriteParameterInitializerScope
(https://cs.chromium.org/chromium/src/v8/src/parsing/pattern-rewriter.cc?rcl=0...).
But the others are trickier.

They have to do with the fact that complex parameters can each get their own
scopes. Example:

"sloppy mode";
function f(a = eval("var x = 1; x"), b = eval("var y = 2; y")) {
}

Each of these param initializers get their own "varblock" scope (see
https://cs.chromium.org/chromium/src/v8/src/parsing/parser.cc?rcl=0&l=4717), and
we need to rescope anything in that particular initializer into that scope.

I'm trying to think about whether we could use more checkpoints in more places
to try to make this work (that is, a checkpoint per function parameter), but I
don't think that'll work for arrow functions:

var f = (a = eval("var x = 1; x"), b = eval("var y = 2; y")) => {};

Note that we don't know that a's initializer needs to have its own scope until
we see the arrow. Would we store a checkpoint per possible param?

[Toon Verwaest, 2016-07-21 05:29:53]

As we discussed offline yesterday, the easiest is probably to do what you
suggest: store bookmarks per possible parameter. However we only need to
actually store the info surrounding parameters containing a sloppy-eval call.
This is in practice much cheaper zone-wise. I'll give that a try...

[nickie, 2016-07-21 15:08:51]

nikolaos@chromium.org changed reviewers:
+ nikolaos@chromium.org

[nickie, 2016-07-21 15:08:52]

LGTM (for all that it's worth), modulo a comment asking for a DCHECK.

I found the code related to the re-scoping of siblings in
Scope::Snapshot::Rescope quite hard to read.  Maybe it can be simplified a bit.

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

https://codereview.chromium.org/2166023002/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:382: new_parent->inner_scope_ = new_parent->sibling_;
As you're blindly assigning here, I'd recommend to DCHECK that the new_parent
doesn't have any inner scopes already.

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

After chatting with Toon I've taken this one over at
https://codereview.chromium.org/2171703004/

[Toon Verwaest, 2016-07-23 08:34:23]

Description was changed from

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

BUG=v8:5209
==========

to

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

[superseded by adam's CL]

BUG=v8:5209
==========