Fix ProtoBuf packing for kotlin unsigned types
ProtoBuf's ProtoPacked annotation only worked for the original signed primitives and ByteArray, and did not work for unsigned variants.
@KosmX, thanks for opening the PR!
Related issue: https://github.com/Kotlin/kotlinx.serialization/issues/3045
Before moving further with this PR (for the part where inline value classes support is fixed) we need to address the questions from https://github.com/Kotlin/kotlinx.serialization/issues/3045#issuecomment-3211256099
Okay, I'm not a Kotlin maintainer or anything, but here are my thoughts: I'll start with the last, that's probably the easiest or most important
Kotlin might start supporting multi-field value classes at some point, it's worth considering them when addressing single-field value classes issue
As long as value classes use their default serializer, I think, they should be inlined completely:
For example I have a value class Complex(val real: Double, val imaginary: Double), I want it to be extremely compact in a huge array.
And that's the same for serialization, especially for binary formats like protobuf.
And, if for some reason, I do not want to serialize the value class this way, I can still go the long way, and write a custom serializer, or maybe we can add an annotation for serializable value classes (like DontInlineSerialize)
If we want to match standards/specifications exactly, don't bend protobuf's rules, maybe do the inlining the opposite way: inline only for unsigned primitives, and for explicit SerializeInline annotation.
Currently we try to inline things in all formats.
potentially, there might be the same issues for inline values classes that are fields of regular classes, we have to figure out something there too
The way I edited the isPackable getter, it is recursive on nested value classes. I should write a test-case for that.
we need to figure out how to deal with Proto<Something>-annotated fields of inline value classes
If the value class is inlined, I would look for these annotations recursively, from the deepest.
On the implementation question, I have some ideas, but nothing precise-enough, and I think it would be better to have the complete specification first, maybe involve more people, ask what their thoughts are.
Hi @KosmX! I think we can merge this without solving all the issues mentioned. Do you want to continue working with this PR? If so, I'll submit my portion of comments