Implement message set wire format #836
Conversation
| continue outer; | ||
| } | ||
| } else { | ||
| throw UnsupportedError('Invalid message set item (tag = $tagNumber)'); |
There was a problem hiding this comment.
I wonder if it is better to ignore the extra field here.
I wonder what other implementations do.
Probably OK to throw. We don't have anywhere to put unknown fields.
There was a problem hiding this comment.
Yeah we could ignore the field as the encoding is basically a repeated message encoding, and when decoding messages extra fields are always OK.
OTOH if there's an extra field here it probably means some kind of non-standard behavior or a bug in the program generating the message.
No strong opinions either way..
|
One concern here is binary sizes and runtime performance when the new class is not used. For binary sizes I checked a large internal dart2js app that currently does not use message sets. The binary size increased 0.007% so that should be OK. I don't know yet why the size increased though. Because the difference is so small I suspect it may just be some non-determinism in compilation. For runtime performance I checked the benchmarks in the repo. The binaries before and after this PR are identical, so it seems like at least for small apps that don't use the feature there will be no difference in generated code and runtime performance. |
Message set wire format is old and deprecated format used instead of extensions
in ancient protos.
This adds a new
GeneratedMessagesubclass$_MessageSetto override binaryserialization and deserialization methods for message sets. Message set
messages now extend
$_MessageSetinstead ofGeneratedMessage.The new class is prefixed as
$_to make the addition backwards compatible.