mojo bindings: Support enums with no values

mojo bindings: Support enums with no values

Previously it was invalid to define an empty enum, i.e.:

  enum A {};

Now empty enums are a valid declaration. In practice it's not useful to
have an empty, non-extensible enum because all values will fail message
validation. However, an extensible empty enum may be useful because all
 values are valid for it.

BUG=619693
TEST=existing tests & new bindings conformance test

Review-Url: https://codereview.chromium.org/2741943002
Cr-Commit-Position: refs/heads/master@{#456596}
Committed: https://chromium.googlesource.com/chromium/src/+/497ec8da95fdb294957278ebd8dc49d5f88b980f

[watk, 2017-03-10 07:15:49]

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

[commit-bot: I haz the power, 2017-03-10 07:15:54]

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

[watk, 2017-03-10 07:21:06]

watk@chromium.org changed reviewers:
+ yzshen@chromium.org

[watk, 2017-03-10 07:21:06]

Hi yzshen@, PTAL. Let me know if I can add more tests. I figured that most of it
is covered by existing tests.

Thanks

[watk, 2017-03-10 07:22:14]

Description was changed from

==========
mojo bindings: Support enums with no values

Previously it was invalid to define an empty enum:

  enum A {};

This CL makes it a valid declaration. In practice it's not useful to
have an empty, non-extensible enum because all values will fail message
validation. However, all values are valid for an extensible enum.

BUG=619693
TEST=existing tests & new bindings conformance test
==========

to

==========
mojo bindings: Support enums with no values

Previously it was invalid to define an empty enum, i.e.:

  enum A {};

Now empty enums are a valid declaration. In practice it's not useful to
have an empty, non-extensible enum because all values will fail message
validation. However, all values are valid for an extensible enum.

BUG=619693
TEST=existing tests & new bindings conformance test
==========

[watk, 2017-03-10 07:22:56]

Description was changed from

==========
mojo bindings: Support enums with no values

Previously it was invalid to define an empty enum, i.e.:

  enum A {};

Now empty enums are a valid declaration. In practice it's not useful to
have an empty, non-extensible enum because all values will fail message
validation. However, all values are valid for an extensible enum.

BUG=619693
TEST=existing tests & new bindings conformance test
==========

to

==========
mojo bindings: Support enums with no values

Previously it was invalid to define an empty enum, i.e.:

  enum A {};

Now empty enums are a valid declaration. In practice it's not useful to
have an empty, non-extensible enum because all values will fail message
validation. However, an extensible empty enum may be useful because all
 values are valid for it.

BUG=619693
TEST=existing tests & new bindings conformance test
==========

[commit-bot: I haz the power, 2017-03-10 07:26:27]

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

[commit-bot: I haz the power, 2017-03-10 07:26:28]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[yzshen1, 2017-03-10 16:43:48]

LGTM with one nit.

Thanks!

https://codereview.chromium.org/2741943002/diff/1/mojo/public/interfaces/bind...
File
mojo/public/interfaces/bindings/tests/data/validation/conformance_mthd21_empty_enum_accepts_any_value.data
(right):

https://codereview.chromium.org/2741943002/diff/1/mojo/public/interfaces/bind...
mojo/public/interfaces/bindings/tests/data/validation/conformance_mthd21_empty_enum_accepts_any_value.data:1:
[dist4]message_header  // num_bytes
nit: it would be nice to have "extensible" in the file name. Because by default
empty enum rejects any value.

[optional] maybe we could add a test for the non-extensiable empty enum as well.

[watk, 2017-03-13 00:36:24]

Patchset #2 (id:20001) has been deleted

[watk, 2017-03-13 00:39:32]

Thanks! New test added and I had to special case the operator<<() implementation
for empty enums because MSVC doesn't like switches with no cases.

The template change was pretty trivial but I'll ask for another lgtm from you in
case I missed something.

https://codereview.chromium.org/2741943002/diff/1/mojo/public/interfaces/bind...
File
mojo/public/interfaces/bindings/tests/data/validation/conformance_mthd21_empty_enum_accepts_any_value.data
(right):

https://codereview.chromium.org/2741943002/diff/1/mojo/public/interfaces/bind...
mojo/public/interfaces/bindings/tests/data/validation/conformance_mthd21_empty_enum_accepts_any_value.data:1:
[dist4]message_header  // num_bytes
On 2017/03/10 16:43:48, yzshen1 wrote:
> nit: it would be nice to have "extensible" in the file name. Because by
default
> empty enum rejects any value.
> 
> [optional] maybe we could add a test for the non-extensiable empty enum as
well.

Both done.

[yzshen1, 2017-03-13 17:44:42]

LGTM++

[watk, 2017-03-14 00:10:39]

The CQ bit was checked by watk@chromium.org

[commit-bot: I haz the power, 2017-03-14 00:11:23]

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-03-14 00:25:31]

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

[commit-bot: I haz the power, 2017-03-14 00:25:32]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[watk, 2017-03-14 00:40:38]

The CQ bit was checked by watk@chromium.org

[commit-bot: I haz the power, 2017-03-14 00:40:52]

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-03-14 02:54:18]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489452038850520,
"parent_rev": "f80d22fd4bcf1fc04feea4644da8df7c73f418f8", "commit_rev":
"497ec8da95fdb294957278ebd8dc49d5f88b980f"}

[commit-bot: I haz the power, 2017-03-14 02:55:01]

Description was changed from

==========
mojo bindings: Support enums with no values

Previously it was invalid to define an empty enum, i.e.:

  enum A {};

Now empty enums are a valid declaration. In practice it's not useful to
have an empty, non-extensible enum because all values will fail message
validation. However, an extensible empty enum may be useful because all
 values are valid for it.

BUG=619693
TEST=existing tests & new bindings conformance test
==========

to

==========
mojo bindings: Support enums with no values

Previously it was invalid to define an empty enum, i.e.:

  enum A {};

Now empty enums are a valid declaration. In practice it's not useful to
have an empty, non-extensible enum because all values will fail message
validation. However, an extensible empty enum may be useful because all
 values are valid for it.

BUG=619693
TEST=existing tests & new bindings conformance test

Review-Url: https://codereview.chromium.org/2741943002
Cr-Commit-Position: refs/heads/master@{#456596}
Committed:
https://chromium.googlesource.com/chromium/src/+/497ec8da95fdb294957278ebd8dc...
==========

[commit-bot: I haz the power, 2017-03-14 02:55:02]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/497ec8da95fdb294957278ebd8dc...