[ignition] Refactor array iterator to separate out access

[ignition] Refactor array iterator to separate out access

Refactors the bytecode array iterator to separate the iteration and the
bytecode parameter access, placing the latter into a separate
super-class. This will allow us to have other forms of access, e.g.
reverse iteration.

Committed: https://crrev.com/ae8a77ea5f3edcd621fb0e7c017af92217a6de94
Cr-Commit-Position: refs/heads/master@{#41152}

[Leszek Swirski, 2016-11-21 12:17:05]

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

[commit-bot: I haz the power, 2016-11-21 12:17:14]

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-11-21 12:19:43]

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

[commit-bot: I haz the power, 2016-11-21 12:19:44]

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_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_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/16524)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)

[Leszek Swirski, 2016-11-21 12:30:59]

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

[commit-bot: I haz the power, 2016-11-21 12:31:03]

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-11-21 12:58:34]

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

[commit-bot: I haz the power, 2016-11-21 12:58:35]

Dry run: This issue passed the CQ dry run.

[Leszek Swirski, 2016-11-21 14:06:27]

leszeks@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[Leszek Swirski, 2016-11-21 14:06:28]

Hey Ross, here's a small refactor which I use for the reverse iterator.

[rmcilroy, 2016-11-21 14:14:27]

LGTM with two comments.

https://codereview.chromium.org/2519923002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-accessor.h (right):

https://codereview.chromium.org/2519923002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-accessor.h:25: bool OffsetInBounds() const;
nit - make this private, and just OffsetIsValid() ? 

Maybe just DCHECK this on SetOffset and have the rest of the code assume it is
valid (no ifs) ?

https://codereview.chromium.org/2519923002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-iterator.h (right):

https://codereview.chromium.org/2519923002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-iterator.h:14: class V8_EXPORT_PRIVATE
BytecodeArrayIterator : public BytecodeArrayAccessor {
nit - final

[Leszek Swirski, 2016-11-21 16:41:58]

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

[commit-bot: I haz the power, 2016-11-21 16:42:00]

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

[Leszek Swirski, 2016-11-21 16:49:03]

https://codereview.chromium.org/2519923002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-accessor.h (right):

https://codereview.chromium.org/2519923002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-accessor.h:25: bool OffsetInBounds() const;
On 2016/11/21 14:14:26, rmcilroy wrote:
> nit - make this private, and just OffsetIsValid() ? 
> 
> Maybe just DCHECK this on SetOffset and have the rest of the code assume it is
> valid (no ifs) ?

yes to private, I want to keep OffsetInBounds though because we're not doing
other validity checks (e.g. is a valid bytecode)

We can't do a DCHECK, because this would break the functionality of
BytecodeArrayIterator.Advance, which can advance the offset past the end of the
array, and relies on this to determine if it's done.

https://codereview.chromium.org/2519923002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-iterator.h (right):

https://codereview.chromium.org/2519923002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-iterator.h:14: class V8_EXPORT_PRIVATE
BytecodeArrayIterator : public BytecodeArrayAccessor {
On 2016/11/21 14:14:26, rmcilroy wrote:
> nit - final

Done.

[commit-bot: I haz the power, 2016-11-21 17:08:02]

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

[commit-bot: I haz the power, 2016-11-21 17:08:02]

Dry run: This issue passed the CQ dry run.

[Leszek Swirski, 2016-11-21 17:16:16]

The CQ bit was checked by leszeks@chromium.org

[Leszek Swirski, 2016-11-21 17:16:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2519923002/#ps40001
(title: "Final class and private function")

[commit-bot: I haz the power, 2016-11-21 17:16:26]

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-11-21 17:18:20]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1479748576647260,
"parent_rev": "407007ca2362f3b059f5431b60e703bb2730ea34", "commit_rev":
"86222ea9d7bffd3d441e159a8b955fb39c771b3f"}

[commit-bot: I haz the power, 2016-11-21 17:18:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-21 17:19:01]

Description was changed from

==========
[ignition] Refactor array iterator to separate out access

Refactors the bytecode array iterator to separate the iteration and the
bytecode parameter access, placing the latter into a separate
super-class. This will allow us to have other forms of access, e.g.
reverse iteration.
==========

to

==========
[ignition] Refactor array iterator to separate out access

Refactors the bytecode array iterator to separate the iteration and the
bytecode parameter access, placing the latter into a separate
super-class. This will allow us to have other forms of access, e.g.
reverse iteration.

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

[commit-bot: I haz the power, 2016-11-21 17:19:02]

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