[feature request] scoped enums with an option

[goldenbull]

Currently pb3 applies c98 enum scoping rule for enums. This is inconvenient when we need same Identifier in different enums. I hope there will be an new option for enums as follow:

enum Status {
  option scoped = true;
  START = 0;
  STOP = 1;
  PAUSE = 2;
}

enum Action {
  option scoped = true;
  NOP = 0;
  START = 1; // note this entry has a different value from Status::START
  STOP = 2;
}

With the annotation option scoped = true the compiler can use enum class for C++11, providing better code, and we users will feel more comfortable to use proper identifiers for enums. And this is totally compatible with current implementation, all existing code will not be disturbed.

[philsc]

👍 I would love to have something like this! Even if I have to specify it as an option to the protoc invocation I'd be happy.

[lostindark]

I like this idea. The c98 enum scoping rule is super annoying when you want to use short enum values. Now you have to add prefix which made the enum very ugly in managed languages like Java/C#.

[goldenbull]

One possible constraint is that some platforms do not have scoped enums support. For example, most embedded compiler do not support C++11 well.

Just for curiosity, besides C98 and object-C, are there any other languages which do not support scoped enums? I googled for some popular languages and seems all of them have scoped syntax.

[lostindark]

The solution to languages that doesn't support scoped enum is allow customize prefix for enum values:
option unscoped_enum_prefix = "eFoo";

[gegles]

👍 I would love to have something like this as well!

[warrenfalk]

Sorry, I deleted my comment, because I think I will try to implement option scoped instead.

But I think the opposite is true. "option scoped" will break existing generators that expect enum values to be unique at the enum's parent's level whereas "option auto_strip_enum_prefixes" can be safely ignored by all generators and then only implemented later. This principle of least breakage was why I had started going that route in the first place.

I've changed my mind only because I think that C++ will be the only generator that breaks with option scoped and I think that I can probably fix it, but I'm worried that the PR won't be accepted.

The question is what the C++ code generation constraints are. Is enum class allowed? Or should the C++ generator automatically add a prefix (the enum's name for example) when option scoped is encountered?

[goldenbull]

For some embeded system, C98 is still widely used, so I think protoc should generate C98 compatible code. One possible choice is that the option scoped forces protoc to generate C++11 code, and user should be aware of this behaviour.

[warrenfalk]

Turns out that current design makes option scoped quite a bit more difficult than it would seem. The problem is that the code that does the validation and throws the error happens prior to the code that interprets the options, and between these two things is an ominous comment that says "Note that the following steps must occur in exactly the specified order". I think that the validation code can be moved to a different function that occurs after parsing, but this will be quite a bit more difficult as the logic in that context will probably have to be rewritten. See the following code:

  // Convert children.
  BUILD_ARRAY(proto, result, message_type, BuildMessage  , NULL);
  BUILD_ARRAY(proto, result, enum_type   , BuildEnum     , NULL);
  BUILD_ARRAY(proto, result, service     , BuildService  , NULL);
  BUILD_ARRAY(proto, result, extension   , BuildExtension, NULL);

The scope validation happens in the BUILD_ARRAY for enum_type above and throws the error

  // Copy options.
  if (!proto.has_options()) {
    result->options_ = NULL;  // Will set to default_instance later.
  } else {
    AllocateOptions(proto.options(), result);
  }

  // Note that the following steps must occur in exactly the specified order.

The above comment seems to rule out any re-ordering of operations

  // Cross-link.
  CrossLinkFile(result, proto);

  // Interpret any remaining uninterpreted options gathered into
  // options_to_interpret_ during descriptor building.  Cross-linking has made
  // extension options known, so all interpretations should now succeed.
  if (!had_errors_) {
    OptionInterpreter option_interpreter(this);
    for (vector<OptionsToInterpret>::iterator iter =
             options_to_interpret_.begin();
         iter != options_to_interpret_.end(); ++iter) {
      option_interpreter.InterpretOptions(&(*iter));
    }
    options_to_interpret_.clear();
  }

We don't have options until this point.

I'm still going to give it a shot, but my C++ knowledge has atrophied over many years of disuse.

[warrenfalk]

I think that "option scoped" should only signify that the .proto writer means the enum value names to be scoped as children of the enum rather than siblings. Then it would be up to each language generator to behave in whatever way is appropriate for its language given this option. For all generators, except C++, this means just using the enum values as they are because most languages already scope the values that way.

For C++ we could generate enum class but I don't think we should unless the user has explicitly opted into C++11 generation, which should be a separate option (probably to protoc as suggested in #67 where xfxyjwf has said that it probably won't happen). So I think the C++ generator should find a way to generate C98 code when the option is present, which I think means prefixing these values with the enum name, as this is the prevailing way to handle this situation when it's handled manually. And then if an option to allow C++11 is added later, this behavior can take that into account.

[lostindark]

The way to generate a C98 code for C++ is to add another option:
option unscoped_enum_prefix = "eFoo";
The name can be cpp_enum_prefix is only C++ is affected by this naming constrain (I'm not quite sure about other languages like Ruby/Go etc).

So it looks like this:

1. No scope and unscoped_enum_prefix.option
   Existing behavior
2. scoped option only
   generated C++11 enum class
3. unscoped_enum_prefix option only
   Add prefix to C++ enum.

People can choose # 2 for newer code with C++11 support, choose # 3 to be compatible with C98 c++ compiler while enjoy the short enum name in other language, and choose # 1 for max compatibility with proto2.
I agree this seems introduce too much options for a simple feature, but I haven't figured out better and easier ways.

[warrenfalk]

Options that are non-language-specific, like scoped should have non-language-specific meanings only, which the language generators should translate into code according to their language's semantics.

The meaning of scoped is that the enum value names are scoped to the enum and not the parent of the enum - this is a language-independent meaning.

A C# or Java generator can just ignore this option as their semantics already have this behavior.

A C++ generator that is limited to C98 just has to find a way to achieve the same purpose within the constraints of C98. A C++ generator that is not limited to C98 (perhaps in the future by default or by option) could use enum class.

In other words, a Java developer that adds option scoped should not have to know that this will make his .proto unusable by embedded C++ projects.

The idea of a cpp_prefix enum-only option seems fine and if present, a C++ generator limited to C98 can add the prefix and ignore scoped.

[goldenbull]

How about option scoped_or_prefixed = true?
This is a language-neutral option. C#/Java will just simply ignore it, and C98 can use enum name as the prefix.