Use the compile-time constant for v8::ValueSerializer format version.

Use the compile-time constant for v8::ValueSerializer format version.

BUG=704293
TBR=jochen@chromium.org

Review-Url: https://codereview.chromium.org/2801053003
Cr-Commit-Position: refs/heads/master@{#469437}
Committed: https://chromium.googlesource.com/chromium/src/+/41a331f01308223da331ecbdf5d73f4fb03fe42e

[jbroman, 2017-04-06 20:27:27]

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

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

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 20:40:54]

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

[commit-bot: I haz the power, 2017-04-06 20:40:55]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jbroman, 2017-04-06 20:49:53]

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

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

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

[jbroman, 2017-05-02 14:47:28]

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

[commit-bot: I haz the power, 2017-05-02 14:47:59]

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-05-02 15:53:41]

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

[commit-bot: I haz the power, 2017-05-02 15:53:42]

Dry run: This issue passed the CQ dry run.

[jbroman, 2017-05-02 15:55:37]

jbroman@chromium.org changed reviewers:
+ jsbell@chromium.org, pwnall@chromium.org

[jsbell, 2017-05-02 16:59:34]

lgtm

[pwnall, 2017-05-02 18:56:52]

On 2017/05/02 16:59:34, jsbell wrote:
> lgtm

LGTM.

I wish there'd be a way to assert that the function will remain constexpr to
avoid static initialization issues. I don't know of such a way, though.

[jbroman, 2017-05-02 19:57:41]

On 2017/05/02 at 18:56:52, pwnall wrote:
> On 2017/05/02 16:59:34, jsbell wrote:
> > lgtm
> 
> LGTM.
> 
> I wish there'd be a way to assert that the function will remain constexpr to
avoid static initialization issues. I don't know of such a way, though.

We could use it in a context that constant-expression is required, like a
static_assert:

static_assert(v8::CurrentValueSerializerFormatVersion(), "V8 should have a
constexpr format version");

I'm not quite sure how much it's worth doing that, but I can add it if you'd
like.

[jbroman, 2017-05-02 19:59:06]

On 2017/05/02 at 19:57:41, jbroman wrote:
> On 2017/05/02 at 18:56:52, pwnall wrote:
> > On 2017/05/02 16:59:34, jsbell wrote:
> > > lgtm
> > 
> > LGTM.
> > 
> > I wish there'd be a way to assert that the function will remain constexpr to
avoid static initialization issues. I don't know of such a way, though.
> 
> We could use it in a context that constant-expression is required, like a
static_assert:
> 
> static_assert(v8::CurrentValueSerializerFormatVersion(), "V8 should have a
constexpr format version");
> 
> I'm not quite sure how much it's worth doing that, but I can add it if you'd
like.

To elaborate slightly, it feels to me a bit like adding an assertion that the
return type of a function doesn't change -- it's generally the responsibility of
someone changing the contract of a function to look at the callers. Besides, we
do have the sizes check for static initializers (after-the-fact).

[jbroman, 2017-05-04 17:34:28]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2017-05-04 17:35:16]

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-05-04 17:44:35]

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

[commit-bot: I haz the power, 2017-05-04 17:44:37]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jbroman, 2017-05-04 18:18:23]

Description was changed from

==========
Use the compile-time constant for v8::ValueSerializer format version.

BUG=704293
==========

to

==========
Use the compile-time constant for v8::ValueSerializer format version.

BUG=704293
TBR=jochen@chromium.org
==========

[jbroman, 2017-05-04 18:18:45]

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

[jbroman, 2017-05-04 18:18:46]

TBR-ing jochen for DEPS change to permit another v8 constant-only header

[jbroman, 2017-05-04 18:59:21]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2017-05-04 18:59:46]

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-05-04 19:54:37]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493924361139920,
"parent_rev": "762ccd490dd28b660ca3a0f9f5befe07e2f82702", "commit_rev":
"41a331f01308223da331ecbdf5d73f4fb03fe42e"}

[commit-bot: I haz the power, 2017-05-04 19:54:47]

Description was changed from

==========
Use the compile-time constant for v8::ValueSerializer format version.

BUG=704293
TBR=jochen@chromium.org
==========

to

==========
Use the compile-time constant for v8::ValueSerializer format version.

BUG=704293
TBR=jochen@chromium.org

Review-Url: https://codereview.chromium.org/2801053003
Cr-Commit-Position: refs/heads/master@{#469437}
Committed:
https://chromium.googlesource.com/chromium/src/+/41a331f01308223da331ecbdf5d7...
==========

[commit-bot: I haz the power, 2017-05-04 19:54:49]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/41a331f01308223da331ecbdf5d7...