Convert public API lists to arrays #1395
Conversation
* A few handwritten classes * Some code generation logic * Some tests
| .beginControlFlow("return %M", Stdlib.buildList) | ||
| .addStatement( | ||
| "this@%L.forEach { element -> add(element.%M(json)) }", | ||
| name, | ||
| schema.modifierToProtocol, | ||
| ) | ||
| .endControlFlow() | ||
| .addCode("⇥.toTypedArray()⇤") |
There was a problem hiding this comment.
Not convinced that just adding this actually helps perf since it's still building a list first. But I think to build an array directly, Modifier would need a size and an iterator.
There was a problem hiding this comment.
We control Modifier so we could add behavior to expose the size if we need to.
| ) | ||
|
|
||
| private object EventSerializer : KSerializer<Event> { |
There was a problem hiding this comment.
Hmm yeah this is unfortunate. I initially envisioned that we would be able to do this with a property-local serializer. But I guess that's too late because the enclosing serializer for the type is what determines whether or not the value needs to be included. Once we're inside a serializer for the property we are always going to write out the value.
I'll look into this more. Maybe ask the serialization folks if they'd be open to a similar annotation to treat arrays as values and not references.
Another potential option in the future is to use a value class when custom equals gets supported. Then we could have a ValueBasedArray wrapper around an array. This assumes that it doesn't somehow incur boxing, which is not yet clear since there's no implementation of this to play with yet.
There was a problem hiding this comment.
I initially envisioned that we would be able to do this with a property-local serializer. But I guess that's too late because the enclosing serializer for the type is what determines whether or not the value needs to be included.
Yep, that appears to be the case.
I filed Kotlin/kotlinx.serialization#2409, let's see what they think.
|
Marking this as blocked and for 1.0 (4-6 months away, minimum). Let's wait and see if serialization delivers. |
Per Jake's comment on #1043. Split from #1392.
The arrays cause problems with some of the serialization tests: empty lists were omitted by default; empty arrays are included by default because different instances of an empty array are not equals.
Currently this PR half-implements a verbose solution – custom serializers for all serializable classes containing an array property. Happy to add the remaining custom serializers if this is the preferred solution. I'm hoping there's a less verbose way though.
Another option is to give these classes singleton default empty array values, but this is brittle because it's easy to write new code generation that doesn't know it should use these singletons. Also adds to the public API in a weird way.