[regexp] Add additional asserts to RegExp builtins

[regexp] Add additional asserts to RegExp builtins

Review-Url: https://codereview.chromium.org/2799663003
Cr-Commit-Position: refs/heads/master@{#44450}
Committed: https://chromium.googlesource.com/v8/v8/+/9d7354f9f3061b2094a1dfe085e340d93e51477e

[jgruber, 2017-04-06 09:09:51]

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

[commit-bot: I haz the power, 2017-04-06 09:09:53]

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

[jgruber, 2017-04-06 09:12:59]

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

[commit-bot: I haz the power, 2017-04-06 09:29:03]

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

[commit-bot: I haz the power, 2017-04-06 09:29:04]

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_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)
  v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
  v8_linux_verify_csa_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng_...)
  v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/25561)
  v8_win64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng_triggered/b...)

[jgruber, 2017-04-06 09:41:19]

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

[commit-bot: I haz the power, 2017-04-06 09:41: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, 2017-04-06 10:19:30]

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

[commit-bot: I haz the power, 2017-04-06 10:19:31]

Dry run: This issue passed the CQ dry run.

[jgruber, 2017-04-06 10:50:14]

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

[Camillo Bruni, 2017-04-06 10:59:27]

LGTM with very minor nits ;)

https://codereview.chromium.org/2799663003/diff/20001/src/builtins/builtins-r...
File src/builtins/builtins-regexp-gen.cc (right):

https://codereview.chromium.org/2799663003/diff/20001/src/builtins/builtins-r...
src/builtins/builtins-regexp-gen.cc:72: CSA_ASSERT(this, HasInstanceType(regexp,
JS_REGEXP_TYPE));
nit: could you use/add IsJSRegexp?

https://codereview.chromium.org/2799663003/diff/20001/src/builtins/builtins-r...
src/builtins/builtins-regexp-gen.cc:831: CSA_ASSERT(this,
HasInstanceType(regexp, JS_REGEXP_TYPE));
ditto

https://codereview.chromium.org/2799663003/diff/20001/src/builtins/builtins-r...
src/builtins/builtins-regexp-gen.cc:1738: CSA_ASSERT(this, IsFastRegExp(context,
regexp));
nit: oneline statement would be fine here.
if (...) CSA_ASSERT(...);

[jgruber, 2017-04-06 14:54:23]

https://codereview.chromium.org/2799663003/diff/20001/src/builtins/builtins-r...
File src/builtins/builtins-regexp-gen.cc (right):

https://codereview.chromium.org/2799663003/diff/20001/src/builtins/builtins-r...
src/builtins/builtins-regexp-gen.cc:72: CSA_ASSERT(this, HasInstanceType(regexp,
JS_REGEXP_TYPE));
On 2017/04/06 10:59:27, Camillo Bruni wrote:
> nit: could you use/add IsJSRegexp?

Done and updated all use sites.

https://codereview.chromium.org/2799663003/diff/20001/src/builtins/builtins-r...
src/builtins/builtins-regexp-gen.cc:831: CSA_ASSERT(this,
HasInstanceType(regexp, JS_REGEXP_TYPE));
On 2017/04/06 10:59:27, Camillo Bruni wrote:
> ditto

Done.

https://codereview.chromium.org/2799663003/diff/20001/src/builtins/builtins-r...
src/builtins/builtins-regexp-gen.cc:1738: CSA_ASSERT(this, IsFastRegExp(context,
regexp));
On 2017/04/06 10:59:27, Camillo Bruni wrote:
> nit: oneline statement would be fine here.
> if (...) CSA_ASSERT(...);

Done.

[jgruber, 2017-04-06 14:55:14]

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

[commit-bot: I haz the power, 2017-04-06 14:55:18]

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, 2017-04-06 14:56:50]

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

[commit-bot: I haz the power, 2017-04-06 14:56:51]

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_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_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_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...)

[jgruber, 2017-04-06 14:58:31]

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

[commit-bot: I haz the power, 2017-04-06 14:58:38]

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

[jgruber, 2017-04-06 15:01:06]

The CQ bit was unchecked by jgruber@chromium.org

[jgruber, 2017-04-06 15:01:14]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2017-04-06 15:01:15]

The patchset sent to the CQ was uploaded after l-g-t-m from cbruni@chromium.org
Link to the patchset: https://codereview.chromium.org/2799663003/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2017-04-06 15:01:22]

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, 2017-04-06 15:10:24]

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

[commit-bot: I haz the power, 2017-04-06 15:10:25]

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/24026)
  v8_linux64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng_triggered...)

[jgruber, 2017-04-06 15:15:20]

The CQ bit was checked by jgruber@chromium.org

[commit-bot: I haz the power, 2017-04-06 15:15:28]

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, 2017-04-06 15:48:02]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491491720694850,
"parent_rev": "2db171c2ba138443b2e177f8d31e9d2ba58c3c65", "commit_rev":
"9d7354f9f3061b2094a1dfe085e340d93e51477e"}

[commit-bot: I haz the power, 2017-04-06 15:48:07]

Description was changed from

==========
[regexp] Add additional asserts to RegExp builtins
==========

to

==========
[regexp] Add additional asserts to RegExp builtins

Review-Url: https://codereview.chromium.org/2799663003
Cr-Commit-Position: refs/heads/master@{#44450}
Committed:
https://chromium.googlesource.com/v8/v8/+/9d7354f9f3061b2094a1dfe085e340d93e5...
==========

[commit-bot: I haz the power, 2017-04-06 15:48:08]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/9d7354f9f3061b2094a1dfe085e340d93e5...