Create ScopeInfos while analyzing the Scope chain

Create ScopeInfos while analyzing the Scope chain

Instead of creating them on demand all over the place.

I plan to link ScopeInfos together, and having one place where all
ScopeInfos are created will make this easier.

R=verwaest@chromium.org,adamk@chromium.org
TBR=mstarzinger@chromium.org
BUG=v8:5215
Committed: https://crrev.com/0c3789fb6a55e5d900986f5172834ad666fcc072
Cr-Commit-Position: refs/heads/master@{#39003}

[jochen (gone - plz use gerrit), 2016-08-26 17:09: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 17:09:26]

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 17:12:56]

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

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

Dry run: Try jobs failed on following builders:
  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/11689)
  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_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)

[jochen (gone - plz use gerrit), 2016-08-26 17:58:16]

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

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

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 18:01:55]

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

[commit-bot: I haz the power, 2016-08-26 18:01:56]

Dry run: Try jobs failed on following builders:
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/7761)

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

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

[commit-bot: I haz the power, 2016-08-26 18:10:59]

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-08-26 18:22:02]

Description was changed from

==========
Create ScopeInfos while analyzing the Scope chain

Instead of creating them on demand all over the place.

I plan to link ScopeInfos together, and having one place where all
ScopeInfos are created will make this easier.

R=verwaest@chromium.org
BUG=
==========

to

==========
Create ScopeInfos while analyzing the Scope chain

Instead of creating them on demand all over the place.

I plan to link ScopeInfos together, and having one place where all
ScopeInfos are created will make this easier.

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

[jochen (gone - plz use gerrit), 2016-08-26 18:22:02]

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

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

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

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

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

ptal

still trying to work out why the bytecode generator doesn't like me

[adamk, 2016-08-26 18:41:08]

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

https://codereview.chromium.org/2281073002/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:1626: scope->AllocateVariablesRecursively(isolate);
For stack-allocated block-scoped variables, this may allocate some variables in
this scope (or an outer scope), so running it after ScopeInfo::Create seems like
it'll just be wrong. But probably this only breaks the debugger, since lazy
compilation wouldn't depend on stack allocated variables from other scopes? We
really ought to have some tests that fail here...

[jochen (gone - plz use gerrit), 2016-08-26 18:51:54]

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

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

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-08-26 18:52:24]

new version that recurses separately for creating scope infos

[commit-bot: I haz the power, 2016-08-26 18:54:07]

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

[commit-bot: I haz the power, 2016-08-26 18:54:08]

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_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_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/11695)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)

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

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

[commit-bot: I haz the power, 2016-08-26 18:55:08]

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 18:57:17]

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

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

Dry run: Try jobs failed on following builders:
  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_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_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)

[jochen (gone - plz use gerrit), 2016-08-28 16:04:22]

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

[commit-bot: I haz the power, 2016-08-28 16:04:32]

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-28 16:07:41]

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

[commit-bot: I haz the power, 2016-08-28 16:07:42]

Dry run: Try jobs failed on following builders:
  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/11715)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/7783)

[jochen (gone - plz use gerrit), 2016-08-28 18:02:51]

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

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

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-28 18:04:44]

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

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

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_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/11656)

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

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

[commit-bot: I haz the power, 2016-08-29 09:07:57]

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-08-29 09:13:43]

jochen@chromium.org changed reviewers:
+ marja@chromium.org

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

+marja

ptal

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

most changes are replacing GetScopeInfo(isolate) with scope_info()

https://codereview.chromium.org/2281073002/diff/120001/src/ast/ast-numbering.cc
File src/ast/ast-numbering.cc (left):

https://codereview.chromium.org/2281073002/diff/120001/src/ast/ast-numbering....
src/ast/ast-numbering.cc:237: if (FLAG_ignition && node->scope() != nullptr &&
no longer needed, as we always allocate the ScopeInfos

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

https://codereview.chromium.org/2281073002/diff/120001/src/ast/scopes.cc#oldc...
src/ast/scopes.cc:820: // 3) Allocate variables.
// Do foo.
foo();

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

https://codereview.chromium.org/2281073002/diff/120001/src/ast/scopes.cc#newc...
src/ast/scopes.cc:1591: scope_info_ = ScopeInfo::Create(info->isolate(), zone(),
this);
the debugger wants all scopes anyways.

https://codereview.chromium.org/2281073002/diff/120001/src/debug/debug-scopes.cc
File src/debug/debug-scopes.cc (right):

https://codereview.chromium.org/2281073002/diff/120001/src/debug/debug-scopes...
src/debug/debug-scopes.cc:118: Scope::Analyze(info.get());
just do the whole thing

[marja, 2016-08-29 09:26:25]

Code lgtm but does this regress?

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

https://codereview.chromium.org/2281073002/diff/120001/src/ast/scopes.cc#newc...
src/ast/scopes.cc:1593: // Allocate variables for inner scopes.
This comment is a copy-paste error.

[commit-bot: I haz the power, 2016-08-29 09:30:08]

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

[commit-bot: I haz the power, 2016-08-29 09:30:09]

Dry run: This issue passed the CQ dry run.

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

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

[commit-bot: I haz the power, 2016-08-29 09:30:27]

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-29 09:54:26]

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

[commit-bot: I haz the power, 2016-08-29 09:54:27]

Dry run: This issue passed the CQ dry run.

[marja, 2016-08-29 10:49:30]

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

https://codereview.chromium.org/2281073002/diff/140001/src/ast/scopes.cc#newc...
src/ast/scopes.cc:1590: DCHECK(scope_info_.is_null());
Discussed offline: maybe we should create ScopeInfos only for those Scopes which
need them, not for all. The chained-together-ScopeInfos will also only contain
those ones which are needed, and the rest will be discarded.

[Toon Verwaest, 2016-08-29 11:46:25]

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

https://codereview.chromium.org/2281073002/diff/140001/src/ast/scopes.cc#newc...
src/ast/scopes.cc:1591: scope_info_ = ScopeInfo::Create(info->isolate(), zone(),
this);
Wut? We pass along info just to get the isolate?

[Toon Verwaest, 2016-08-29 11:48:58]

https://codereview.chromium.org/2281073002/diff/140001/src/debug/debug-scopes.cc
File src/debug/debug-scopes.cc (left):

https://codereview.chromium.org/2281073002/diff/140001/src/debug/debug-scopes...
src/debug/debug-scopes.cc:118: scope->AllocateVariables(info.get());
Can this be private then?
Should Analyze be moved to DeclarationScope?

[jochen (gone - plz use gerrit), 2016-08-29 12:15:25]

ptal

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

https://codereview.chromium.org/2281073002/diff/140001/src/ast/scopes.cc#newc...
src/ast/scopes.cc:1591: scope_info_ = ScopeInfo::Create(info->isolate(), zone(),
this);
On 2016/08/29 at 11:46:25, Toon Verwaest wrote:
> Wut? We pass along info just to get the isolate?

Eventually, I'd like get the ScopeInfo of the outer scope chain from the
ParseInfo, but I can also change Isolate* to ParseInfo* then

[jochen (gone - plz use gerrit), 2016-08-29 12:15:33]

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

[commit-bot: I haz the power, 2016-08-29 12:15:39]

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-29 12:43:07]

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

[commit-bot: I haz the power, 2016-08-29 12:43:08]

Dry run: This issue passed the CQ dry run.

[Toon Verwaest, 2016-08-30 09:13:11]

lgtm

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

The CQ bit was checked by jochen@chromium.org

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

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

[commit-bot: I haz the power, 2016-08-30 09:13:36]

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-30 09:16:25]

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

[commit-bot: I haz the power, 2016-08-30 09:16:26]

Try jobs failed on following builders:
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/22851)

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

The CQ bit was checked by jochen@chromium.org

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

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

[commit-bot: I haz the power, 2016-08-30 09:22:36]

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-30 09:26:25]

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

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

Try jobs failed on following builders:
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/22852)

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

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

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

Michael, mind rubberstamping the compiler/ change?

[jochen (gone - plz use gerrit), 2016-08-30 09:41:43]

Description was changed from

==========
Create ScopeInfos while analyzing the Scope chain

Instead of creating them on demand all over the place.

I plan to link ScopeInfos together, and having one place where all
ScopeInfos are created will make this easier.

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

to

==========
Create ScopeInfos while analyzing the Scope chain

Instead of creating them on demand all over the place.

I plan to link ScopeInfos together, and having one place where all
ScopeInfos are created will make this easier.

R=verwaest@chromium.org,adamk@chromium.org
TBR=mstarzinger@chromium.org
BUG=v8:5215
==========

[jochen (gone - plz use gerrit), 2016-08-30 09:41:54]

The CQ bit was checked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2016-08-30 09:41:55]

tbr'ing since it's a trivial api update

[commit-bot: I haz the power, 2016-08-30 09:42:09]

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-30 09:48:36]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-08-30 09:49:06]

Description was changed from

==========
Create ScopeInfos while analyzing the Scope chain

Instead of creating them on demand all over the place.

I plan to link ScopeInfos together, and having one place where all
ScopeInfos are created will make this easier.

R=verwaest@chromium.org,adamk@chromium.org
TBR=mstarzinger@chromium.org
BUG=v8:5215
==========

to

==========
Create ScopeInfos while analyzing the Scope chain

Instead of creating them on demand all over the place.

I plan to link ScopeInfos together, and having one place where all
ScopeInfos are created will make this easier.

R=verwaest@chromium.org,adamk@chromium.org
TBR=mstarzinger@chromium.org
BUG=v8:5215

Committed: https://crrev.com/0c3789fb6a55e5d900986f5172834ad666fcc072
Cr-Commit-Position: refs/heads/master@{#39003}
==========

[commit-bot: I haz the power, 2016-08-30 09:49:07]

Patchset 10 (id:??) landed as
https://crrev.com/0c3789fb6a55e5d900986f5172834ad666fcc072
Cr-Commit-Position: refs/heads/master@{#39003}

[Michael Starzinger, 2016-09-05 13:43:47]

LGTM.