Thanks for the feedback. I added some background information here: https://github.com/protocolbuffers/protobuf/blob/3.12.x/docs/implementing_proto3_presence.md#background
On Tue, May 5, 2020 at 8:43 PM Nadav Samet <[email protected]> wrote: > The need for non-empty oneof came up in conversations with users. The > other evidence for this being a real user need is that PGV has a validator > for non-empty oneofs > <https://github.com/envoyproxy/protoc-gen-validate#oneofs> in their code > gen, so it wouldn't be unreasonable if someone implemented this via > reflection rather than codegen. > > Totally understand that this may be the safest way to roll this change > out. This is a pretty significant change to proto3, so it might be worth > adding to the changelog the motivation for this change and considerations > that were taken (i.e. possibly wrapper types are not good enough due to the > extra space) > > On Tue, May 5, 2020 at 6:50 PM Josh Haberman <[email protected]> wrote: > >> On Tue, May 5, 2020 at 6:24 PM Nadav Samet <[email protected]> wrote: >> >>> What representations do you mean? In general it is not expected that >>>> oneof names are exposed in wire formats. Authors of .proto files are free >>>> to change them at will without breaking the wire. >>>> >>> >>> An example would be code that produces an alternative JSON >>> representation that groups all the fields of a oneof inside a nested >>> object. For example if a message like >>> message MyMessage { >>> oneof foo { >>> int32 v = 1 >>> } >>> } >>> >>> gets encoded to JSON by user's code as: >>> "my_one_of": {"foo": 5}} >>> >>> Then such code would have to be modified to treat synthetic >>> oneofs differently. >>> >> >> Can you point to code that does this, or is it hypothetical? >> >> I agree that if such a format existed, serialization code would need to >> change to use the "real_" variants of the oneof accessors. But the parsers >> and serializers for all of the real formats we use with reflection >> (protobuf binary, TextFormat, JSON) did not need to change to support >> proto3 optional, which is a significant benefit. I suspect lots of other >> reflection-based algorithms will benefit from this too. >> >> >>> Can you give an example of some of the code that would be affected? >>>> >>> >>> Some users use oneofs to build ADTs, and for them oneofs should never >>> be empty <https://github.com/scalapb/ScalaPB/issues/633>. To enforce >>> this at runtime, they may have written a validator that takes an instance >>> of a message, and uses its descriptor to traverse all the oneofs and fail >>> when it finds a oneof where none of its fields is set. With the proposed >>> change, this validator would have to be changed to only care about >>> non-synthetic oneofs. >>> >> >> This sounds like it is also hypothetical? I think it's helpful to talk >> about real examples when possible. >> >> It's true that in some cases code will need to change to ignore synthetic >> oneofs. But in the cases where we've had to implement this, it is not a >> very intrusive change. It mainly consists of two simple transformations: >> >> 1. f->containing_oneof() -> f->real_containing_oneof() >> 2. f->oneof_decl_count() -> f->real_oneof_decl_count() >> >> I agree it would have been nicer not to have to use synthetic oneofs. But >> there is a *lot* of code out there that manipulates protos using >> reflection. Synthetic oneofs allow many/most of these algorithms to work >> with proto3 optional fields with no modifications. Without them it would be >> a riskier change to roll out, because we don't know how many algorithms >> would simply not be aware that proto3 optional fields exist and need to >> preserve presence. >> > > > -- > -Nadav > -- 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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/protobuf/CAFFe09iFXAtBr4Yuodzc9Mm6-eaep3KHh-Fvp%2ByjpsN%2BR64P1g%40mail.gmail.com.
