A more lightweight, informative error value in TryFrom<i32> for enums

[mzabaluev]

DecodeError as the error type of the generated TryFrom<i32> conversions for enum values is problematic:

• It's overweight for the call sites converting a field value to its representative domain-safe enum, as it's generally meant to describe a decoding failure on a wire message. Construction of a DecodeError value makes an allocation even with a static message.
• It erases information about the raw value, both programmatically (there's only a cause string) and as currently generated, from the displayed message (could use format! in the conversion impls, but that'll cost another allocation).

Add a new UnknownEnumValue error type to prost.
Change the TryFrom<i32> conversions generated for enums to use that as the associated Error type.
The error value carries the original integer as the public inner member.

As this is a breaking change, it's not meant for 0.12, but for a future version.

[caspermeijn]

I understand why you need this. I have two open question:

• Should this be a separated struct or should the i32 be part of DecodeError somehow?
• It seems DecodeError mostly filled with static strings. Do we want to change that to a non_exthaustive enum instead of a string?

[mzabaluev]

I think the TryFrom impl generated for an enum is a separate case from the protobuf message decoding error.
It arises in a different context: the consumer tries to convert a field of an already decoded message to its nominal enum type.
DecodeError, for its part, has all the data to represent a failure arising when a message is decoded.

[caspermeijn]

I think the TryFrom impl generated for an enum is a separate case from the protobuf message decoding error. It arises in a different context: the consumer tries to convert a field of an already decoded message to its nominal enum type. DecodeError, for its part, has all the data to represent a failure arising when a message is decoded.

That makes sense. I do think that DecodeError should be changed to a non_exthaustive enum instead of a string, but that is an issue for another day.

[caspermeijn]

Approved, but needs to wait for the next major release.

Thanks for your contribution.

[caspermeijn]

@mzabaluev The main prost library is moved to a subdirectory. Could you solve the merge conflict?

[mzabaluev]

The main prost library is moved to a subdirectory. Could you solve the merge conflict?

It's easy to do, but I will wait on #1068 to make sure tests pass.

[caspermeijn]

It's easy to do, but I will wait on #1068 to make sure tests pass.

That PR is merged now

[mzabaluev]

@mzabaluev The main prost library is moved to a subdirectory. Could you solve the merge conflict?

Done against 683091a

[caspermeijn]

Thanks for your contribution