Chain ScopeInfos together

Chain ScopeInfos together

This will allow getting the entire scope chain from a SharedFunctionInfo
which in turn will allow for generating bytecode when we just have the
SFI

R=verwaest@chromium.org,marja@chromium.org
BUG=v8:5215
Committed: https://crrev.com/ce3f46b172d56cb0badffe975207fd971e9f39a5
Cr-Commit-Position: refs/heads/master@{#39243}

[jochen (gone - plz use gerrit), 2016-08-24 12:58:55]

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

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

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-08-24 13:13:39]

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

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

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)

[jochen (gone - plz use gerrit), 2016-08-26 10:56:24]

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

[commit-bot: I haz the power, 2016-08-26 10:56:29]

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-08-26 10:57:39]

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

[commit-bot: I haz the power, 2016-08-26 10:57:40]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/11663)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/22679)

[jochen (gone - plz use gerrit), 2016-08-26 11:04:49]

ready for review now

[jochen (gone - plz use gerrit), 2016-08-26 11:04:55]

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

[commit-bot: I haz the power, 2016-08-26 11:05:04]

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-08-26 11:48:44]

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

[commit-bot: I haz the power, 2016-08-26 11:48:45]

Dry run: Try jobs failed on following builders:
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/11607)
  v8_linux_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng_triggered/b...)

[jochen (gone - plz use gerrit), 2016-08-26 11:53:32]

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

[commit-bot: I haz the power, 2016-08-26 11:53:34]

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

[Toon Verwaest, 2016-08-26 12:04:37]

lgtm

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

https://codereview.chromium.org/2271993002/diff/60001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:838: if (scope->NeedsContext()) return scope;
I guess this happens for outer function scopes from which no locals are
captured?

[Toon Verwaest, 2016-08-26 12:07:01]

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

https://codereview.chromium.org/2271993002/diff/60001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:835: if (scope->scope_type() == SCRIPT_SCOPE &&
scope->scope_info_.is_null()) {
How does this work for let variables in top-level in a top-level or eager
compile? We should also have scope info in that case right?

[commit-bot: I haz the power, 2016-08-26 12:26:44]

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

[commit-bot: I haz the power, 2016-08-26 12:26:45]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2016-08-26 13:14:11]

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

https://codereview.chromium.org/2271993002/diff/60001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:835: if (scope->scope_type() == SCRIPT_SCOPE &&
scope->scope_info_.is_null()) {
On 2016/08/26 at 12:07:01, Toon Verwaest wrote:
> How does this work for let variables in top-level in a top-level or eager
compile? We should also have scope info in that case right?

For a top-level script, we'll invoke GetScopeInfo() on the outer most scope, so
scope_info_ will be non-NULL for any of the inner scopes that go up to this.

https://codereview.chromium.org/2271993002/diff/60001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:838: if (scope->NeedsContext()) return scope;
On 2016/08/26 at 12:04:37, Toon Verwaest wrote:
> I guess this happens for outer function scopes from which no locals are
captured?

I guess in practice, this should be always true.

[marja, 2016-08-29 07:53:00]

otherwise lgtm

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

https://codereview.chromium.org/2271993002/diff/60001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:829: while (scope) {
Style nit: while (scope != nullptr) seems to be the common way.

https://codereview.chromium.org/2271993002/diff/60001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:835: if (scope->scope_type() == SCRIPT_SCOPE &&
scope->scope_info_.is_null()) {
On 2016/08/26 13:14:11, jochen wrote:
> On 2016/08/26 at 12:07:01, Toon Verwaest wrote:
> > How does this work for let variables in top-level in a top-level or eager
> compile? We should also have scope info in that case right?
> 
> For a top-level script, we'll invoke GetScopeInfo() on the outer most scope,
so
> scope_info_ will be non-NULL for any of the inner scopes that go up to this.
> 

I don't get this answer at all..

[jochen (gone - plz use gerrit), 2016-09-06 08:26:14]

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

[commit-bot: I haz the power, 2016-09-06 08:26:17]

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

[jochen (gone - plz use gerrit), 2016-09-06 08:29:23]

jochen@chromium.org changed reviewers:
+ bmeurer@chromium.org, mstarzinger@chromium.org

[jochen (gone - plz use gerrit), 2016-09-06 08:29:24]

ptal Marja - more or less rewritten.

Now with DCHECKs that verify that the scope info chains are correct while
deserializing the context chain.

Benedikt, please stamp debug/

Michael, since you were interested in this, fyi

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

https://codereview.chromium.org/2271993002/diff/140001/src/ast/scopes.cc#newc...
src/ast/scopes.cc:376: deserialization_mode ==
DeserializationMode::kKeepScopeInfo,
if we deserialize off heap, we nuke the scope_info_, so the DCHECKS won't work

https://codereview.chromium.org/2271993002/diff/140001/src/ast/scopes.cc#newc...
src/ast/scopes.cc:380: outer_scope->scope_info_->Equals(
the same context might have different scope infos: if we compile a function once
with FCG and once with TF, the scope pushed on the stack will have scope infos
from different asts. Therefore, we need to use a deep-equals method instead of
just comparing the pointers.

[jochen (gone - plz use gerrit), 2016-09-06 08:31:18]

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

[commit-bot: I haz the power, 2016-09-06 08:31:21]

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-09-06 08:48:14]

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

[commit-bot: I haz the power, 2016-09-06 08:48:16]

Dry run: Try jobs failed on following builders:
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/8260)
  v8_mac_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng_triggered/bui...)

[marja, 2016-09-06 08:49:27]

I was about to l-g-t-m this change, but then noticed that tryjobs aren't happy
:( (ie it does look good, no idea why it doesn't work)

[jochen (gone - plz use gerrit), 2016-09-07 08:50:48]

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

[commit-bot: I haz the power, 2016-09-07 08:50:51]

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

[jochen (gone - plz use gerrit), 2016-09-07 09:09:25]

now with greener trybots!

[commit-bot: I haz the power, 2016-09-07 09:18:20]

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

[commit-bot: I haz the power, 2016-09-07 09:18:21]

Dry run: This issue passed the CQ dry run.

[marja, 2016-09-07 10:29:48]

lgtm

[jgruber, 2016-09-07 10:36:53]

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

[jgruber, 2016-09-07 10:36:54]

LGTM for debug/

[jochen (gone - plz use gerrit), 2016-09-07 10:51:26]

The CQ bit was checked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2016-09-07 10:51:27]

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

[commit-bot: I haz the power, 2016-09-07 10:51:38]

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-09-07 10:53:58]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-09-07 10:54:29]

Description was changed from

==========
Chain ScopeInfos together

This will allow getting the entire scope chain from a SharedFunctionInfo
which in turn will allow for generating bytecode when we just have the
SFI

R=verwaest@chromium.org,marja@chromium.org
BUG=v8:5215
==========

to

==========
Chain ScopeInfos together

This will allow getting the entire scope chain from a SharedFunctionInfo
which in turn will allow for generating bytecode when we just have the
SFI

R=verwaest@chromium.org,marja@chromium.org
BUG=v8:5215

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

[commit-bot: I haz the power, 2016-09-07 10:54:31]

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