Store NonLocals in variables_

Store NonLocals in variables_

Now that ordered_variables_ is used to find non-dynamic variables, and NonLocals are always stored in the scope that introduces them, we can rely on variables_ to also cache non-locals. This has 2 advantages:
1) we don't need DynamicScopePart anymore, reducing all scopes by a pointer
2) upon second lookup of a non-local we don't need to walk the entire chain anymore. The cached value will immediately be found.

BUG=v8:5209
Committed: https://crrev.com/3a9c7b55af27105c482e9449911b010f46830cb4
Cr-Commit-Position: refs/heads/master@{#38855}

[Toon Verwaest, 2016-08-23 21:06:44]

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

[Toon Verwaest, 2016-08-23 21:06:45]

ptal

[adamk, 2016-08-23 22:12:20]

In your CL description, you say that ordered_variables_ is used to "find
non-dynamic variables". But LookupLocal() still looks in variables_. Given that
LookupLocal is API used by the Parser, I'm not completely sure this is safe. Is
the idea that variables_ only contains non-locals once the Parser is done?

https://codereview.chromium.org/2276483003/diff/1/src/ast/scopes.cc
File src/ast/scopes.cc (left):

https://codereview.chromium.org/2276483003/diff/1/src/ast/scopes.cc#oldcode1180
src/ast/scopes.cc:1180: Indent(n1, "// dynamic vars:\n");
So this means non-locals will now be printed as part of local vars? That
seems...not ideal for debugging. How much code would it be to retain some
separation when printing?

[Toon Verwaest, 2016-08-23 23:50:38]

Addressed printing comment.

Sorry, I didn't mean find in the LookupLocal sense, but for variable allocation.
They won't be "allocated" since that uses ordered_variables_ to which dynamics
aren't added.

Indeed, we only fill in nonlocals once parsing finishes. One kind of "non-local"
was already added to variables_: DeclareDynamicGlobal-declared variables. Now
this is extended to all other dynamics.

[adamk, 2016-08-24 00:02:24]

lgtm

Thanks for the explanation. I'm sympathetic to not wanting to spend an extra
pointer for this. And it seems highly unlikely we'd move NonLocal creation any
earlier than it is now, so this seems safe enough.

I wish there was some more separation, but I guess the high level hope is to
stop calling LookupLocal from the Parser?

[Toon Verwaest, 2016-08-24 04:45:21]

Indeed. As described in the doc, I'm also thinking of resolving variables as we
close scopes; although I'm not 100% sure yet how that will work for non-locals.
This step (plus the previous move of NonLocal allocation into LookupRecurisive) 
makes it a little clearer what's needed to resolve variables in a scope.

[Toon Verwaest, 2016-08-24 04:47:36]

The CQ bit was checked by verwaest@chromium.org

[Toon Verwaest, 2016-08-24 04:47:36]

The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2276483003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-08-24 04:47:40]

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-08-24 08:51:59]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-24 08:52:12]

Description was changed from

==========
Store NonLocals in variables_

Now that ordered_variables_ is used to find non-dynamic variables, and NonLocals
are always stored in the scope that introduces them, we can rely on variables_
to also cache non-locals. This has 2 advantages:
1) we don't need DynamicScopePart anymore, reducing all scopes by a pointer
2) upon second lookup of a non-local we don't need to walk the entire chain
anymore. The cached value will immediately be found.

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

to

==========
Store NonLocals in variables_

Now that ordered_variables_ is used to find non-dynamic variables, and NonLocals
are always stored in the scope that introduces them, we can rely on variables_
to also cache non-locals. This has 2 advantages:
1) we don't need DynamicScopePart anymore, reducing all scopes by a pointer
2) upon second lookup of a non-local we don't need to walk the entire chain
anymore. The cached value will immediately be found.

BUG=v8:5209

Committed: https://crrev.com/3a9c7b55af27105c482e9449911b010f46830cb4
Cr-Commit-Position: refs/heads/master@{#38855}
==========

[commit-bot: I haz the power, 2016-08-24 08:52:13]

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