Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++11 enum class #3625

Closed
denisiussion opened this issue Sep 12, 2017 · 5 comments
Closed

C++11 enum class #3625

denisiussion opened this issue Sep 12, 2017 · 5 comments

Comments

@denisiussion
Copy link

It would be great use 'enum class' in the C++11 code generation.

@hadrielk
Copy link

yes it would be awesome, but it sounds like it would be surprisingly hard to achieve: see #1079

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Sep 12, 2017

@hadrielk It depends on what's your goal. If you want to allow such proto definition:

// proto file
enum Foo {
  UNKNOWN = 0;
}
enum Bar {
  UNKNOWN = 0;
}

this is not possible even with C++11 enum class, because protobuf internally flattens the name scope and the two UNKNOWN enum values conflict with each other.

However, if you just want to make protoc generate C++11 enum classes, that should be pretty easy after C++11 is required to build protobuf.

@hadrielk
Copy link

So I guess the advantage with that latter case is you'd get to avoid implicit type conversion when it's used in the generated accessor functions? That's good sure. But yeah for me the goal is to use the same enumerator names in different enums in the same namespace/package. 😢

@hadrielk
Copy link

Out of curiosity, is it "flattened" in the sense that the reflection/descriptor stuff can't deal with it being scoped, or do you just mean in the outputted .h/.cc files it's flattened (e.g., in the const variables created for each enumerator name and such)

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Sep 13, 2017

It's the former. When we store enum values internally in descriptors, the names we use in indices are flattened (i.e., both Foo.UNKNOWN and Bar.UNKNOWN are indexed by the name "UNKNOWN"), and this is exposed in public APIs such as FileDescriptor::FindEnumValueByName():
https://github.com/google/protobuf/blob/master/src/google/protobuf/descriptor.h#L1327

If we are to allow two enums to define the same enum value name, all such FindEnumValueByName() APIs will be broken and it will have unpleasant consequences.

It's probably possible to deprecate all such FindEnumValueByName() methods (and all similar enum value name related APIs in every protobuf implementation) and change the behavior after all usage is gone, but that will take time and work. It's hardly worthwhile, I would say.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants