search

awakesecurity / proto3-suite

Haskell Protobuf Implementation
https://hackage.haskell.org/package/proto3-suite
Other
81 stars 56 forks source link

Please add optional back to the AST #229

Open xeinherjar opened 1 year ago

xeinherjar commented 1 year ago

It looks like Optional was removed in this PR: https://github.com/awakesecurity/proto3-suite/pull/165/files

The reason given was that it wasn't supported in proto3, but it is in fact part of the spec, see: https://protobuf.dev/programming-guides/proto3/#specifying-field-rules

Specifying Field Rules 
Message fields can be one of the following:

singular: a well-formed message can have zero or one of this field (but not more than one). When using proto3 syntax, this is the default field rule when no other field rules are specified for a given field. You cannot determine whether it was parsed from the wire. It will be serialized to the wire unless it is the default value. For more on this subject, see [Field Presence](https://protobuf.dev/programming-guides/field_presence).
optional: the same as singular, except that you can check to see if the value was explicitly set. An optional field is in one of two possible states:
the field is set, and contains a value that was explicitly set or parsed from the wire. It will be serialized to the wire.
the field is unset, and will return the default value. It will not be serialized to the wire.
repeated: this field type can be repeated zero or more times in a well-formed message. The order of the repeated values will be preserved.
map: this is a paired key/value field type. See [Maps](https://protobuf.dev/programming-guides/encoding#maps) for more on this field type.

It was erroneously left out of early documentation per: https://github.com/protocolbuffers/protobuf/issues/10463

Can that PR be reverted?

gbaz commented 1 year ago

I don't think putting back optional on its own suffices. The spec gives a new behavior to it, which we'd have to take into account in the code as well!

atriantafyllos-da commented 1 month ago

Is there any update on this issue? Is there a plan to support the optional fields by proto3-suite?

riz0id commented 4 weeks ago

Is there any update on this issue? Is there a plan to support the optional fields by proto3-suite?

I have tried within the past 6 months and adding optional back to the AST would minimally require a complete rework of how the protobuf AST is set up in proto3-suite. There are no plans to do this as of now.

I should also note that in addition to reworking the AST it would also be necessary to add compilation procedures handling optional to the proto3-suite Haskell code generator. This in turn would require new serialization functions for messages with the optional modifier.

ixmatus commented 4 weeks ago

@atriantafyllos-da we also welcome PRs if you feel inclined to try tackling it. We're pretty low on bandwidth right now at $job so you can't expect a lot of work except for that which affects us directly.

If you do want to try tackling this, I think taking some time to understand the codebase, what @riz0id has said, and writing a small proposal/design for how to add the feature would be a great way to start. A design to review would be a lot easier on other maintainers with experience in the codebase to collaborate over.