Behold, a unit test for Scanner::BookmarkScope (& scanner bookmarking).

Behold, a unit test for Scanner::BookmarkScope (& scanner bookmarking).

This is in preparation for upcmoming scanner + bookmarking cleanups.

Also, drive-by fix for setting a bookmark close to the end of the stream,
when the look-ahead character (c0_) is kEndOfInput, which the bookmarking
logic also used as kNoBookmark.

R=marja@chomium.org
BUG=v8:4947
Committed: https://crrev.com/d8eeaed3f95c1645d60b1bdd93c17792b70cbe87
Cr-Commit-Position: refs/heads/master@{#39507}

[vogelheim, 2016-09-16 13:23:54]

ptal

[vogelheim, 2016-09-16 13:24:09]

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

[commit-bot: I haz the power, 2016-09-16 13:24:13]

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-16 13:49:34]

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

[commit-bot: I haz the power, 2016-09-16 13:49:35]

Dry run: This issue passed the CQ dry run.

[vogelheim, 2016-09-19 11:37:35]

vogelheim@chromium.org changed reviewers:
+ marja@chromium.org
- marja@chomium.org

[marja, 2016-09-19 11:50:39]

lgtm

https://codereview.chromium.org/2345053003/diff/1/test/cctest/parsing/test-sc...
File test/cctest/parsing/test-scanner.cc (right):

https://codereview.chromium.org/2345053003/diff/1/test/cctest/parsing/test-sc...
test/cctest/parsing/test-scanner.cc:48: for (size_t bookmark_pos = 1;
bookmark_pos < tokens.size() - 1;
You could also set the bookmark to pos 0 without scanning anything, no?

https://codereview.chromium.org/2345053003/diff/1/test/cctest/parsing/test-sc...
test/cctest/parsing/test-scanner.cc:48: for (size_t bookmark_pos = 1;
bookmark_pos < tokens.size() - 1;
Can you also set it to the end? Or does that not make sense?

[vogelheim, 2016-09-19 16:04:46]

https://codereview.chromium.org/2345053003/diff/1/test/cctest/parsing/test-sc...
File test/cctest/parsing/test-scanner.cc (right):

https://codereview.chromium.org/2345053003/diff/1/test/cctest/parsing/test-sc...
test/cctest/parsing/test-scanner.cc:48: for (size_t bookmark_pos = 1;
bookmark_pos < tokens.size() - 1;
On 2016/09/19 11:50:39, marja wrote:
> You could also set the bookmark to pos 0 without scanning anything, no?

Done.

[My thinking was that this doesn't test anything useful, since that equates a
normal pass through the stream, without any actual 'reset' going on. I guess it
does test one more edge case, though.]

https://codereview.chromium.org/2345053003/diff/1/test/cctest/parsing/test-sc...
test/cctest/parsing/test-scanner.cc:48: for (size_t bookmark_pos = 1;
bookmark_pos < tokens.size() - 1;
On 2016/09/19 11:50:39, marja wrote:
> Can you also set it to the end? Or does that not make sense?

Done.

[vogelheim, 2016-09-19 16:05:45]

The CQ bit was checked by vogelheim@chromium.org

[vogelheim, 2016-09-19 16:05:45]

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/2345053003/#ps40001
(title: "Adapt comment to previous change.")

[commit-bot: I haz the power, 2016-09-19 16:05:54]

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-19 16:07:37]

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

[commit-bot: I haz the power, 2016-09-19 16:07:38]

Try jobs failed on following builders:
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/12987)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/12980)
  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_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/24366)

[vogelheim, 2016-09-19 16:24:53]

The CQ bit was checked by vogelheim@chromium.org

[vogelheim, 2016-09-19 16:24:54]

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/2345053003/#ps60001
(title: "Rebase & fix .gyp file.")

[commit-bot: I haz the power, 2016-09-19 16:24:58]

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-19 16:50:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-19 16:51:17]

Description was changed from

==========
Behold, a unit test for Scanner::BookmarkScope (& scanner bookmarking).

This is in preparation for upcmoming scanner + bookmarking cleanups.

Also, drive-by fix for setting a bookmark close to the end of the stream,
when the look-ahead character (c0_) is kEndOfInput, which the bookmarking
logic also used as kNoBookmark.

R=marja@chomium.org
BUG=v8:4947
==========

to

==========
Behold, a unit test for Scanner::BookmarkScope (& scanner bookmarking).

This is in preparation for upcmoming scanner + bookmarking cleanups.

Also, drive-by fix for setting a bookmark close to the end of the stream,
when the look-ahead character (c0_) is kEndOfInput, which the bookmarking
logic also used as kNoBookmark.

R=marja@chomium.org
BUG=v8:4947

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

[commit-bot: I haz the power, 2016-09-19 16:51:18]

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