Issue 515 in protobuf: More intelligent enums

[prot...@googlecode.com]

Status: New
Owner: liu...@google.com
Labels: Type-Defect Priority-Medium

New issue 515 by peterhan...@yahoo.com: More intelligent enums
http://code.google.com/p/protobuf/issues/detail?id=515

(this is a RFE, not a defect)

Currently if you define:

enum Foo {
NONE=0;
FOO1=1;
}
enum Bar {
NONE=0;
BAR1=1;
}


it won't compile because the same enum value, NONE, is used in two
different enums within the same proto file.

This is by design: Protobuf recognizes that the above construct will not
work in C++ and therefore rejects it (no matter the output language). See
Issue #12. The reason for the name collision is due to the somewhat odd
implementation of enums in C++.

For other languages, like Java, there is no name conflict in the generated
code so it is unfair to punish all other languages for the shortcomings of
C++.

The existing restriction is extremely annoying as there aren't really any
good alternatives. Each alternative has its own problems. (alternatives
would be : use one proto file per enum, embed enums inside a message, etc)

Let's be constructive: The big question is how to improve on this without
breaking the millions of lines of C++ code that depend on the existing
behavior?

This group discussion has the solution:
https://groups.google.com/forum/#!topic/protobuf/d-AqClgnDKM (alopecoid's
suggestion is the one I like, it is clean, clear and doesn't break any
existing code)

I'll just re-iterate alopecoid's suggestion here.
The basic suggestion is to add an option to enums:

enum Foo {
option use_namespace = true;
NONE=0;
FOO1=1;
}
enum Bar {
option use_namespace = true;
NONE=0;
BAR1=1;
}


This option would default to 'false', meaning the exact behavior of
protobuf compiler today. However if the option is true the compiler will no
longer fault on the above construct (for any output language). For C++ it
will then generate C++ code like this:


namespace Foo {
enum Enum {
NONE = 0;
FOO1 = 1;
}
}

namespace Bar {
enum Enum {
NONE = 0;
BAR1 = 1;
}
}


and the enums would then be referenced in C++ code like Foo::NONE and
Bar::NONE.

For other languages that also use unscoped enums (like C++) the generated
code for such language will have to find their own solution to the problem.
(are there any such languages at all?)

For languages that use scoped enums (most languages that I can think of,
example: Java) the option would have no effect. It is the same code being
generated in both cases.

I like the suggestion because it doesn't break any existing code.





--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[Michael Haberler]

I like it too, also because it resolves an issue I have with wrapping existing objects - assume you have some C typedefs

typedef enum { FOO=0, BAR=1 } obj1_t;
typedef enum { BAZ=2, XXX=3 } obj2_t;

then I cant have a proto enum like so:

enum ObjTypes {
FOO = 0;
BAR = 1;
BAZ = 2;
XXX = 3;
}

without renaming the enums in the proto enum; the namespace proposal would solve that issue with no apparent downside

- Michael

>
>
>
>
>
> --
> You received this message because this project is configured to send all issue notifications to this address.
> You may adjust your notification preferences at:
> https://code.google.com/hosting/settings
>
> --

> You received this message because you are subscribed to the Google Groups "Protocol Buffers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to protobuf+u...@googlegroups.com.
> To post to this group, send email to prot...@googlegroups.com.
> Visit this group at http://groups.google.com/group/protobuf?hl=en.
> For more options, visit https://groups.google.com/groups/opt_out.
>

[prot...@googlecode.com]

Comment #1 on issue 515 by joew...@live.com: More intelligent enums
http://code.google.com/p/protobuf/issues/detail?id=515

I would say the option name is a little language specific, but the idea is
good. Maybe call it "scoped=true" or something similar and add a
command-line protoc flag to make it the default. I would also like to see
the option to generate C++11 enum classes too, so the namespace feature
would just be used for backwards compatibility for older C++ compilers.

[prot...@googlecode.com]

Comment #2 on issue 515 by xiaof...@google.com: More intelligent enums
http://code.google.com/p/protobuf/issues/detail?id=515

The proposed solution doesn't work because:
1. The name of the enum type is changed from "Foo" to "Foo::Enum" which
will be confusing to the users.
2. For enums nested in a message you simply can't nest a namespace in a
class.

The current scoping semantics is a design choice made a long time ago and
the value of allowing duplicated enum value names is also very limited. It
doesn't seem to be a worthwhile effort to me.

[prot...@googlecode.com]

Comment #3 on issue 515 by kara...@gmail.com: More intelligent enums
http://code.google.com/p/protobuf/issues/detail?id=515

Instead of `namespace Bar { enum ...`, we could use `enum class Bar ...',
or perhaps `struct Bar { enum ...`.

For myself, I don't care much about duplicated enum value names, but I
would certainly like to use scoped enums for style and clarity reasons.

I suppose the reasons for adding something like this to protobuf would be
similar to the reasons for why `enum class` was recently added to C++.

[prot...@googlecode.com]

Comment #4 on issue 515 by xiaof...@google.com: More intelligent enums
http://code.google.com/p/protobuf/issues/detail?id=515

I think the support for C++11 enum class is a good feature to have. We'll
need to wait for some time though, as we currently still prohibit C++11
features in protobuf.

[prot...@googlecode.com]

Comment #5 on issue 515 by cbsm...@gmail.com: More intelligent enums
http://code.google.com/p/protobuf/issues/detail?id=515

Why not put it in now with a flag or an #ifdef on the C++ version?

[prot...@googlecode.com]

Comment #6 on issue 515 by xiaof...@google.com: More intelligent enums
http://code.google.com/p/protobuf/issues/detail?id=515

Disallowing C++11 features is more of a policy issue rather than a
technical one. The policy will eventually change but so far I haven't seen
any move in that direction.

[prot...@googlecode.com]

Comment #7 on issue 515 by peterhan...@yahoo.com: More intelligent enums
http://code.google.com/p/protobuf/issues/detail?id=515

>> The proposed solution doesn't work because:
>> 1. The name of the enum type is changed from "Foo" to "Foo::Enum" which
>> will be confusing to the users.

The proposal text says the new option is .. well, optional. So if you
don't specify it protobuf compiler will (regardless of target language)
generate exactly same code as today. How can that be confusing ? .. except
for those who explicitly put in the new proposed option in the .proto
file .. but then those people have made a conscious choice to use the new
proposed feature.

>> 2. For enums nested in a message you simply can't nest a namespace in a
>> class.

Not sure I understand.

In any case, if you really believe so, we could say in the documentation
that the proposed new option has no effect for C++ and make sure that's
what is happening. This would still bring great benefits to anyone else
(most languages has the notion of namespace for enums) just not to C++
users.

There are already options in .proto files that only benefit some languages
but not others so going down that route is not something new. It all
depends if the design idea of protobuf is to be bound by the lowest common
denominator, in this case C++ ? I certainly understand and appreciate the
desire to keep protobuf design lean and mean so I would sympathize
(somewhat reluctantly :-)) with such a conscious choice.

>> the value of allowing duplicated enum value names is also very limited.
>> It doesn't seem to be a worthwhile effort to me.

What? Perhaps we're just different but we run into these issues all the
time in particular with enum values such
as "YES", "NO", "NONE, "UNDEFINED", words that are likely to be used in
different enums. Yes, there are ways around it .. but they just make
protobuf code harder to read and make your setup more convoluted than what
it needs to. And I love protobuf for the exact opposite reasons.

Cheers.

[Feng Xiao]

On Fri, Jan 31, 2014 at 4:36 AM, <prot...@googlecode.com> wrote:

Comment #7 on issue 515 by peterhan...@yahoo.com: More intelligent enums
http://code.google.com/p/protobuf/issues/detail?id=515

The proposed solution doesn't work because:
1. The name of the enum type is changed from "Foo" to "Foo::Enum" which will be confusing to the users.

The proposal text says the new option is  .. well, optional. So if you don't specify it protobuf compiler will (regardless of target language) generate exactly same code as today. How can that be confusing ? .. except for those who explicitly put in the new proposed option in the .proto file .. but then those people have made a conscious choice to use the new proposed feature.

When enabling this option, the enum type will be named as Foo::Enum while normal people will expect it to be Foo (and indeed it's Foo in any other language). This is the kind of inconsistency we should avoid.

 

2. For enums nested in a message you simply can't nest a namespace in a class.

Not sure I understand.

In any case, if you really believe so,  we could say in the documentation that the proposed new option has no effect for C++ and make sure that's what is happening. This would still bring great benefits to anyone else (most languages has the notion of namespace for enums) just not to C++ users.

There are already options in .proto files that only benefit some languages but not others so going down that route is not something new. It all depends if the design idea of protobuf is to be bound by the lowest common denominator, in this case C++ ? I certainly understand and appreciate the desire to keep protobuf design lean and mean so I would sympathize (somewhat reluctantly :-)) with such a conscious choice.

the value of allowing duplicated enum value names is also very limited. It doesn't seem to be a worthwhile effort to me.

What?  Perhaps we're just different but we run into these issues all the time in particular with enum values such as "YES", "NO", "NONE, "UNDEFINED", words that are likely to be used in different enums. Yes, there are ways around it .. but they just make protobuf code harder to read and make your setup more convoluted than what it needs to. And I love protobuf for the exact opposite reasons.

C++ has such a limitation for many years and we live with it very well. There are well established coding styles requiring a prefix to enum values. I honestly don't believe it makes the code harder to read.

 

Cheers.

--
You received this message because this project is configured to send all issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

--

You received this message because you are subscribed to the Google Groups "Protocol Buffers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to protobuf+unsubscribe@googlegroups.com.

To post to this group, send email to prot...@googlegroups.com.

Visit this group at http://groups.google.com/group/protobuf.

[woodjoe]

The option suggested above could actually just append a prefix to the enum names that is an uppercase, underscore delimited name of the type.

There should at least be the option to generate the enums that are locally scoped.  Right now the only language that doesn't support it is an old version of C++ (C++03).  The generated code in other languages is inconsistent with the rest of the code and makes code unreadable because the enum needs to be scoped once with the type and again using the prefix.

Thanks

Joe

[prot...@googlecode.com]

Comment #8 on issue 515 by wonhee.jung: More intelligent enums
https://code.google.com/p/protobuf/issues/detail?id=515

Isn't it common practice to add UNKNOWN to be enum's 0th field to allow
other side to pick it up as default value when there is a enum value that
isn't their side but added in serialized message? This will instantly
become issue when you add more than one UNKNOWN field in the enum that
sibling each other.

Wrapping enum with another message looks ugly to me, and only simple way
for this so far would be adding prefix for it, like

enum CURRENCY {
UNKNOWN = 0;
NONE = 1;
USD = 2;
// etc...
}

. Is there any other simple way that recently introduced in Protobuf?

[prot...@googlecode.com]

Comment #9 on issue 515 by peterhan...@yahoo.com: More intelligent enums
https://code.google.com/p/protobuf/issues/detail?id=515

>> Wrapping enum with another message looks ugly to me, and only simple way
>> for this so far would be adding prefix for it, like

>> enum CURRENCY {
>> UNKNOWN = 0;
>> NONE = 1;
>> USD = 2;
>> // etc...
>> }

Don't you mean that a workaround would be to add prefix like this:

enum CURRENCY {
CURRENCY_UNKNOWN = 0;
CURRENCY_NONE = 1;
CURRENCY_USD = 2;
// etc...