Reduce number of scope() accesses

Reduce number of scope() accesses

To be able to lazily allocate BlockScopes, we need to reduce access to scope(). The explicit accesses removed here just need access to the current scope to allocate unresolved variable proxies. Those in the future should go over ScopeState rather than directly to Scope.

BUG=v8:5209
Committed: https://crrev.com/8cdb64d82c200b29a8987b3047070ef66ff1744e
Cr-Commit-Position: refs/heads/master@{#38456}

[Toon Verwaest, 2016-08-08 15:40:13]

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

[Toon Verwaest, 2016-08-08 15:40:13]

ptal

[adamk, 2016-08-08 15:55:44]

lgtm once the param_block/init_block thing is dealt with.

https://codereview.chromium.org/2224843003/diff/1/src/parsing/parser.cc
File src/parsing/parser.cc (right):

https://codereview.chromium.org/2224843003/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:4779: if (!parameter.is_simple() &&
scope()->calls_sloppy_eval()) {
Looks like this isn't really related to this change, but rather is a followup to
previous BlockState work. I'm OK with landing it in this patch if you prefer,
but just wanted to make sure my understanding of it being separate is correct.

This is now calling calls_sloppy_eval() on a different scope than it was before.
It's still correct, since param_scope is marked with calls_sloppy_eval, but what
we're really trying to test is if the above code ran.

How about testing that directly?

if (param_block != init_block) { ... }

[Toon Verwaest, 2016-08-08 18:16:02]

https://codereview.chromium.org/2224843003/diff/1/src/parsing/parser.cc
File src/parsing/parser.cc (right):

https://codereview.chromium.org/2224843003/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:4779: if (!parameter.is_simple() &&
scope()->calls_sloppy_eval()) {
On 2016/08/08 15:55:44, adamk wrote:
> Looks like this isn't really related to this change, but rather is a followup
to
> previous BlockState work. I'm OK with landing it in this patch if you prefer,
> but just wanted to make sure my understanding of it being separate is correct.
> 
> This is now calling calls_sloppy_eval() on a different scope than it was
before.
> It's still correct, since param_scope is marked with calls_sloppy_eval, but
what
> we're really trying to test is if the above code ran.
> 
> How about testing that directly?
> 
> if (param_block != init_block) { ... }

You are right, this is an independent change. And the comment to that change is
valid. I'll upload a separate CL.

[Toon Verwaest, 2016-08-08 18:20:17]

The CQ bit was checked by verwaest@chromium.org

[Toon Verwaest, 2016-08-08 18:20:17]

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/2224843003/#ps20001
(title: "Undo unrelated change")

[commit-bot: I haz the power, 2016-08-08 18:20:28]

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-08 18:53:14]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-08 18:58:23]

Description was changed from

==========
Reduce number of scope() accesses

To be able to lazily allocate BlockScopes, we need to reduce access to scope().
The explicit accesses removed here just need access to the current scope to
allocate unresolved variable proxies. Those in the future should go over
ScopeState rather than directly to Scope.

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

to

==========
Reduce number of scope() accesses

To be able to lazily allocate BlockScopes, we need to reduce access to scope().
The explicit accesses removed here just need access to the current scope to
allocate unresolved variable proxies. Those in the future should go over
ScopeState rather than directly to Scope.

BUG=v8:5209

Committed: https://crrev.com/8cdb64d82c200b29a8987b3047070ef66ff1744e
Cr-Commit-Position: refs/heads/master@{#38456}
==========

[commit-bot: I haz the power, 2016-08-08 18:58:24]

Patchset 2 (id:??) landed as
https://crrev.com/8cdb64d82c200b29a8987b3047070ef66ff1744e
Cr-Commit-Position: refs/heads/master@{#38456}