I was thinking that I could use EnumTraits<FooEnum, MappedFooEnum>::FromMojom() to convert FIELD_1 to its corresponding value in MappedFooEnum in the constructor of BarStruct. But I then realized that it still doesn't feel right:

EnumTraits<>::FromMojom() is used for de-seralization. Therefore, it could include validation logic that only applies when the value is received from a message pipe, but not when a local struct is constructed. For example,

// Typemapped to "MappedFooEnum":
//  MappedFooEnum {
//    MAPPED_INVALID_VALUE,
//    MAPPED_VALUE_1,
//    MAPPED_VALUE_2
//  } 
enum FooEnum {
  INVALID_VALUE,
  VALUE_1,
  VALUE_2
};

Struct BarStruct {
  FooEnum = INVALID_VALUE
};

It is conceivable that INVALID_VALUE is not allowed when you pass a FooEnum over the wire, so you write the following EnumTraits:

bool EnumTraits<FooEnum, MappedFooEnum>::FromMojom(FooEnum input, MappedFooEnum* output) {
  // Always reject an incoming INVALID_VALUE
  if (input == INVALID_VALUE)
    return false;
  ...
};

Considering all these, I lean towards not supporting default value when an enum is typemapped. WDYT?

Using EnumTraits seems reasonable to me. Validation logic should in
practice only cause the traits to return false from Read right? i.e. we
definitely don't want traits DCHECKing or anything like that.

If the EnumTraits returns false, what should we use to initialize the field?

We don't require the EnumTraits to always update the output field on failure. During deserialization, we always discard the output on failure so whether it is touched doesn't matter.

It seems "local type conversion" and "deserializing a value off the wire" are two different things. It might be confusing if we use EnumTraits for both purposes.

WDYT? Thanks!

Hmmm.  Forbidding defaults in mojo will mean that if any language has a typemap, all other languages will need a typemap in order to specify the defaults.  So on an overall consistency basis, I don't think that's the right choice.  And I wouldn't expect that an option listed in the .mojom file would be invalid coming over the wire.  

Having said that, from my perspective as a pure C++ consumer, I'm fine with it :-}.

I'll also note that I'd expect *Traits to be fairly liberal in what it accepts, with security or similar restrictions implemented in the service implementation (as those would be variable depending on the source of the request).  Do you have an example of a case in which *Traits be be likely to return false for a default value?

> And I wouldn't expect that an option listed in the .mojom file would be invalid coming over the wire.  

I think it is not impossible. A value is defined as an invalid initial state, and the user would like to enforce that this value is never used in messages.

An example that is not directly related but I think it is kind of similar:


Struct Foo {};
Struct Bar {
  Foo foo;
};

The generated code initializes |foo| as null, but because |foo| is defined as non-nullable. It is illegal if you send a Bar struct with a null |foo| field over the wire.

> Do you have an example of a case in which *Traits be be likely to return false for a default value?
I could look and see whether I could find one. :)

> > Do you have an example of a case in which *Traits be be likely to return false for a default value?
> I could look and see whether I could find one. :)
Okay, the answer is that I couldn't find any real-world example in our codebase.

I think I am no longer against the idea of supporting this. :) I will put together a patch. And I will CHECK if EnumTraits<>::FromMojom returns false. WDYT?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc674b50f259153e15e9f1df37b320f4ffdb41bc

commit dc674b50f259153e15e9f1df37b320f4ffdb41bc
Author: yzshen <yzshen@chromium.org>
Date: Fri Jun 30 00:33:01 2017

Mojo C++ bindings: support setting default value for fields whose types are typemapped enums.

BUG= 738142 

Review-Url: https://codereview.chromium.org/2965663002
Cr-Commit-Position: refs/heads/master@{#483561}

[modify] https://crrev.com/dc674b50f259153e15e9f1df37b320f4ffdb41bc/mojo/public/cpp/bindings/lib/bindings_internal.h
[modify] https://crrev.com/dc674b50f259153e15e9f1df37b320f4ffdb41bc/mojo/public/cpp/bindings/tests/struct_traits_unittest.cc
[modify] https://crrev.com/dc674b50f259153e15e9f1df37b320f4ffdb41bc/mojo/public/interfaces/bindings/tests/struct_with_traits.mojom
[modify] https://crrev.com/dc674b50f259153e15e9f1df37b320f4ffdb41bc/mojo/public/tools/bindings/generators/mojom_cpp_generator.py