[build] Add more v8 build options and fix existing ones.

[build] Add more v8 build options and fix existing ones.

New options: v8_enable_trace_maps, v8_enable_v8_checks.
Renamed options for consistency: v8_object_print to v8_enable_object_print.
Fixed options: v8_enable_verify_heap, v8_enable_object_print.

BUG=

Review-Url: https://codereview.chromium.org/2625393003
Cr-Commit-Position: refs/heads/master@{#42338}
Committed: https://chromium.googlesource.com/v8/v8/+/b90822950004553ecaef48ea04977c8f14bc013e

[Igor Sheludko, 2017-01-13 11:47:41]

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

[Igor Sheludko, 2017-01-13 11:47:42]

PTAL

[Igor Sheludko, 2017-01-13 11:56:31]

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

[commit-bot: I haz the power, 2017-01-13 11:56:38]

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-01-13 12:32:01]

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

[commit-bot: I haz the power, 2017-01-13 12:32:02]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2017-01-13 14:19:26]

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

[jochen (gone - plz use gerrit), 2017-01-13 14:19:27]

deferring to machenbach

[Michael Achenbach, 2017-01-13 14:34:06]

Thanks for making those properly overridable! A nit suggestion and a comment.

https://codereview.chromium.org/2625393003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2625393003/diff/1/BUILD.gn#newcode61
BUILD.gn:61: v8_enable_v8_checks = ""
How about just calling it v8_enable_checks just like the define?

https://codereview.chromium.org/2625393003/diff/1/BUILD.gn#newcode102
BUILD.gn:102: if (v8_object_print == "") {
object_print, enable_disassembler and trace_maps were enabled by default in all
debug builds by the code in line 356 on the left. Now they will only be enabled
in non-optimized debug builds.

Is that really what we want? How about removing the && !v8_optimized_debug here
to not change semantics now?

[Igor Sheludko, 2017-01-13 16:05:00]

https://codereview.chromium.org/2625393003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2625393003/diff/1/BUILD.gn#newcode61
BUILD.gn:61: v8_enable_v8_checks = ""
On 2017/01/13 14:34:06, Michael Achenbach wrote:
> How about just calling it v8_enable_checks just like the define?

Given the flags naming scheme (v8_enable_blah_blah) the v8_enable_checks will
sound like this flag controls all the checks in V8 while it controls checks in
v8.h only. I'd leave it as is.

https://codereview.chromium.org/2625393003/diff/1/BUILD.gn#newcode102
BUILD.gn:102: if (v8_object_print == "") {
On 2017/01/13 14:34:06, Michael Achenbach wrote:
> object_print, enable_disassembler and trace_maps were enabled by default in
all
> debug builds by the code in line 356 on the left. Now they will only be
enabled
> in non-optimized debug builds.
> 
> Is that really what we want? How about removing the && !v8_optimized_debug
here
> to not change semantics now?

Good point. Done.

[Igor Sheludko, 2017-01-13 16:05:05]

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

[commit-bot: I haz the power, 2017-01-13 16:05:08]

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

[Michael Achenbach, 2017-01-13 16:17:49]

lgtm

https://codereview.chromium.org/2625393003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2625393003/diff/1/BUILD.gn#newcode61
BUILD.gn:61: v8_enable_v8_checks = ""
On 2017/01/13 16:04:59, Igor Sheludko wrote:
> On 2017/01/13 14:34:06, Michael Achenbach wrote:
> > How about just calling it v8_enable_checks just like the define?
> 
> Given the flags naming scheme (v8_enable_blah_blah) the v8_enable_checks will
> sound like this flag controls all the checks in V8 while it controls checks in
> v8.h only. I'd leave it as is.

Acknowledged.

[Igor Sheludko, 2017-01-13 16:19:52]

Description was changed from

==========
[build] Add more v8 build options and fix existing.

New options: v8_enable_trace_maps, v8_enable_v8_checks.
Fixed options: v8_enable_verify_heap, v8_object_print.

BUG=
==========

to

==========
[build] Add more v8 build options and fix existing.

New options: v8_enable_trace_maps, v8_enable_v8_checks.
Renamed options for consistency: v8_object_print to v8_enable_object_print.
Fixed options: v8_enable_verify_heap, v8_enable_object_print.

BUG=
==========

[Igor Sheludko, 2017-01-13 16:20:35]

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

[commit-bot: I haz the power, 2017-01-13 16:20:41]

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

[Igor Sheludko, 2017-01-13 16:21:03]

Description was changed from

==========
[build] Add more v8 build options and fix existing.

New options: v8_enable_trace_maps, v8_enable_v8_checks.
Renamed options for consistency: v8_object_print to v8_enable_object_print.
Fixed options: v8_enable_verify_heap, v8_enable_object_print.

BUG=
==========

to

==========
[build] Add more v8 build options and fix existing ones.

New options: v8_enable_trace_maps, v8_enable_v8_checks.
Renamed options for consistency: v8_object_print to v8_enable_object_print.
Fixed options: v8_enable_verify_heap, v8_enable_object_print.

BUG=
==========

[Michael Achenbach, 2017-01-13 16:25:01]

still lgtm

https://codereview.chromium.org/2625393003/diff/40001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2625393003/diff/40001/BUILD.gn#newcode55
BUILD.gn:55: v8_enable_object_print = ""
ok as it is consistent with the other flags now. But now it's inconsistent with
gyp and Makefile. But that's legacy and might not live too much longer...

[commit-bot: I haz the power, 2017-01-13 16:58:55]

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

[commit-bot: I haz the power, 2017-01-13 16:58:56]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2017-01-13 17:26:43]

The CQ bit was checked by ishell@chromium.org

[commit-bot: I haz the power, 2017-01-13 17:26:56]

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-01-13 17:28:52]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484328403807580,
"parent_rev": "57a87a52700deffdcb89d19de4d5163c4e6878fd", "commit_rev":
"b90822950004553ecaef48ea04977c8f14bc013e"}

[commit-bot: I haz the power, 2017-01-13 17:28:57]

Description was changed from

==========
[build] Add more v8 build options and fix existing ones.

New options: v8_enable_trace_maps, v8_enable_v8_checks.
Renamed options for consistency: v8_object_print to v8_enable_object_print.
Fixed options: v8_enable_verify_heap, v8_enable_object_print.

BUG=
==========

to

==========
[build] Add more v8 build options and fix existing ones.

New options: v8_enable_trace_maps, v8_enable_v8_checks.
Renamed options for consistency: v8_object_print to v8_enable_object_print.
Fixed options: v8_enable_verify_heap, v8_enable_object_print.

BUG=

Review-Url: https://codereview.chromium.org/2625393003
Cr-Commit-Position: refs/heads/master@{#42338}
Committed:
https://chromium.googlesource.com/v8/v8/+/b90822950004553ecaef48ea04977c8f14b...
==========

[commit-bot: I haz the power, 2017-01-13 17:28:59]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/b90822950004553ecaef48ea04977c8f14b...