[Interpreter] Avoid accessing Isolate from during bytecode generation.

[Interpreter] Avoid accessing Isolate from during bytecode generation.

Removes all accesses to the Isolate during bytecode generation and the
bytecode pipeline. Adds an DisallowIsolateAccessScope which is used to
enforce this invariant within the BytecodeGenerator.

BUG=v8:5203
Committed: https://crrev.com/1c2c2f43cd61f143558b9b507b78d2b4aba497a7
Cr-Commit-Position: refs/heads/master@{#38716}

[rmcilroy, 2016-08-15 16:38:27]

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

[commit-bot: I haz the power, 2016-08-15 16:38:31]

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

[rmcilroy, 2016-08-15 16:38:48]

rmcilroy@chromium.org changed reviewers:
+ klaasb@google.com, mstarzinger@chromium.org

[rmcilroy, 2016-08-15 16:38:48]

Klaas / Michi, PTAL, thanks.

[commit-bot: I haz the power, 2016-08-15 17:05:21]

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

[commit-bot: I haz the power, 2016-08-15 17:05:22]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2016-08-16 09:10:22]

Looking good, just one question.

https://codereview.chromium.org/2242193002/diff/1/src/ast/scopes.h
File src/ast/scopes.h (right):

https://codereview.chromium.org/2242193002/diff/1/src/ast/scopes.h#newcode431
src/ast/scopes.h:431: Handle<ScopeInfo> scope_info() {
nit: Can we move this to below the declration of "GetScopeInfo" so that the two
are grouped together?

https://codereview.chromium.org/2242193002/diff/1/src/interpreter/bytecode-ge...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2242193002/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:664: class DisallowIsolateAccessScope {
Just for my own clarification mostly: How much worse would it be to remove the
Isolate field from the BytecodeGenerator and explicitly pass it into
FinalizeBytecode instead. Would that allow us to get rid of this helper class
and the "isolate_access_allowed" flag? Feel free to disregard in case I am
missing something obvious.

[klaasb, 2016-08-16 10:55:00]

lgtm

[rmcilroy, 2016-08-16 11:24:55]

https://codereview.chromium.org/2242193002/diff/1/src/ast/scopes.h
File src/ast/scopes.h (right):

https://codereview.chromium.org/2242193002/diff/1/src/ast/scopes.h#newcode431
src/ast/scopes.h:431: Handle<ScopeInfo> scope_info() {
On 2016/08/16 09:10:21, Michael Starzinger wrote:
> nit: Can we move this to below the declration of "GetScopeInfo" so that the
two
> are grouped together?

Done.

https://codereview.chromium.org/2242193002/diff/1/src/interpreter/bytecode-ge...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2242193002/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:664: class DisallowIsolateAccessScope {
On 2016/08/16 09:10:22, Michael Starzinger wrote:
> Just for my own clarification mostly: How much worse would it be to remove the
> Isolate field from the BytecodeGenerator and explicitly pass it into
> FinalizeBytecode instead. Would that allow us to get rid of this helper class
> and the "isolate_access_allowed" flag? Feel free to disregard in case I am
> missing something obvious.

That works, done.

[Michael Starzinger, 2016-08-16 12:26:50]

LGTM. Thanks! Just nits.

https://codereview.chromium.org/2242193002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2242193002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:673: isolate_access_allowed_(true),
nit: Field should no longer be required. Lets drop it.

https://codereview.chromium.org/2242193002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.h (right):

https://codereview.chromium.org/2242193002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.h:215: bool isolate_access_allowed_;
nit: Field should no longer be required. Lets drop it.

[rmcilroy, 2016-08-16 14:22:30]

https://codereview.chromium.org/2242193002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2242193002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:673: isolate_access_allowed_(true),
On 2016/08/16 12:26:50, Michael Starzinger wrote:
> nit: Field should no longer be required. Lets drop it.

Oops, done.

https://codereview.chromium.org/2242193002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.h (right):

https://codereview.chromium.org/2242193002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.h:215: bool isolate_access_allowed_;
On 2016/08/16 12:26:50, Michael Starzinger wrote:
> nit: Field should no longer be required. Lets drop it.

Done.

[rmcilroy, 2016-08-18 11:24:52]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-08-18 11:24:53]

The patchset sent to the CQ was uploaded after l-g-t-m from klaasb@google.com,
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2242193002/#ps40001
(title: "Remove unused field.")

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

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-18 11:26:29]

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

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

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_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_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_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/11132)
  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_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...)
  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_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/11077)
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/22014)

[rmcilroy, 2016-08-18 12:58:06]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-08-18 12:58:07]

The patchset sent to the CQ was uploaded after l-g-t-m from klaasb@google.com,
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2242193002/#ps60001
(title: "Rebase")

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

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-18 13:42:09]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-18 13:42:29]

Description was changed from

==========
[Interpreter] Avoid accessing Isolate from during bytecode generation.

Removes all accesses to the Isolate during bytecode generation and the
bytecode pipeline. Adds an DisallowIsolateAccessScope which is used to
enforce this invariant within the BytecodeGenerator.

BUG=v8:5203
==========

to

==========
[Interpreter] Avoid accessing Isolate from during bytecode generation.

Removes all accesses to the Isolate during bytecode generation and the
bytecode pipeline. Adds an DisallowIsolateAccessScope which is used to
enforce this invariant within the BytecodeGenerator.

BUG=v8:5203

Committed: https://crrev.com/1c2c2f43cd61f143558b9b507b78d2b4aba497a7
Cr-Commit-Position: refs/heads/master@{#38716}
==========

[commit-bot: I haz the power, 2016-08-18 13:42:31]

Patchset 4 (id:??) landed as
https://crrev.com/1c2c2f43cd61f143558b9b507b78d2b4aba497a7
Cr-Commit-Position: refs/heads/master@{#38716}