On Tue, May 5, 2020 at 6:24 PM Nadav Samet <thesa...@gmail.com> 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.

-- 
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 protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/CAFFe09jO-Fpqup4k_U%3DKkpkzDTqD1YOpYiOpt37S6xxdo3Rs9w%40mail.gmail.com.

Reply via email to