generate assoc. consts in enums representing protobuf enum aliases, resolves #792 #849
Conversation
There was a problem hiding this comment.
This changes makes sense to me.
- I think the PR title should be something like:
Add support for enum aliases. - You need to add some tests for the new use cases.
- Does prost need to check for
allow_aliasoption? Ifprotocthrow an error whenallow_aliasis not set, then we don't have to.
prost-build/src/code_generator.rs
Outdated
| self.push_indent(); | ||
| self.buf.push_str("/// Aliases.\n"); | ||
| } | ||
| for variant in &aliases { |
There was a problem hiding this comment.
Personally, I would place the for loop inside the if-block
prost-build/src/code_generator.rs
Outdated
|
|
||
| if aliases.len() > 0 { | ||
| self.push_indent(); | ||
| self.buf.push_str("/// Aliases.\n"); |
There was a problem hiding this comment.
Three slashes is for rustdoc comments. This is not a doc comment, right?
| if !numbers.insert(value.number()) { | ||
| continue; | ||
| for m in &mappings { | ||
| if m.proto_number == value.number() { |
There was a problem hiding this comment.
genereated_names can be used to do this lookup in one step, right?
There was a problem hiding this comment.
I don't see how, that would require a lookup by value instead of name, no?
There was a problem hiding this comment.
You are right. The HashMap is the other way around. Please disregard my comment.
| panic!("Generated enum variant names overlap: `{}` variant name to be used both by `{}` and `{}` ProtoBuf enum values", | ||
| generated_variant_name, old_v, value.name()); | ||
| // if alias ends up being a duplicate, we don't need it, and can skip it. | ||
| // TODO: check if enum values are actually the same |
There was a problem hiding this comment.
I think this TODO needs to be solved
There was a problem hiding this comment.
I feel like duplicates should always be disallowed. Even if they are an alias, then overlap is not disirable.
| if !numbers.insert(value.number()) { | ||
| continue; | ||
| for m in &mappings { | ||
| if m.proto_number == value.number() { |
There was a problem hiding this comment.
You are right. The HashMap is the other way around. Please disregard my comment.
| panic!("Generated enum variant names overlap: `{}` variant name to be used both by `{}` and `{}` ProtoBuf enum values", | ||
| generated_variant_name, old_v, value.name()); | ||
| // if alias ends up being a duplicate, we don't need it, and can skip it. | ||
| // TODO: check if enum values are actually the same |
There was a problem hiding this comment.
I feel like duplicates should always be disallowed. Even if they are an alias, then overlap is not disirable.
This PR adds associated constants to enum
impls representing enum aliases in protocol buffer enums as shown below. The aliases can be referenced by prost when used as default values, causing compilation to fail, see #792.