I tried to serialize an inlined class. The class is implemented in the commons project of a MPP. The compiler arg "-XXLanguage:+InlineClasses" is defined in the JVM and JS project.
Class:
@Serializable
inline class DeviceId(val value: String) {
override fun toString(): String = value
}
And get the following exception:
The root cause was thrown at: ImplementationBodyCodegen.java:863
at org.jetbrains.kotlin.codegen.CompilationErrorHandler.lambda$static$0(CompilationErrorHandler.java:24)
at org.jetbrains.kotlin.codegen.PackageCodegenImpl.generate(PackageCodegenImpl.java:74)
at org.jetbrains.kotlin.codegen.DefaultCodegenFactory.generatePackage(CodegenFactory.kt:97)
at org.jetbrains.kotlin.codegen.DefaultCodegenFactory.generateModule(CodegenFactory.kt:68)
at org.jetbrains.kotlin.codegen.KotlinCodegenFacade.doGenerateFiles(KotlinCodegenFacade.java:47)
at org.jetbrains.kotlin.codegen.KotlinCodegenFacade.compileCorrectFiles(KotlinCodegenFacade.java:39)
at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.generate(KotlinToJVMBytecodeCompiler.kt:446)
at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.compileModules$cli(KotlinToJVMBytecodeCompiler.kt:142)
at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecute(K2JVMCompiler.kt:161)
at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecute(K2JVMCompiler.kt:57)
at org.jetbrains.kotlin.cli.common.CLICompiler.execImpl(CLICompiler.java:96)
at org.jetbrains.kotlin.cli.common.CLICompiler.execImpl(CLICompiler.java:52)
at org.jetbrains.kotlin.cli.common.CLITool.exec(CLITool.kt:93)
at org.jetbrains.kotlin.incremental.IncrementalJvmCompilerRunner.runCompiler(IncrementalJvmCompilerRunner.kt:362)
at org.jetbrains.kotlin.incremental.IncrementalJvmCompilerRunner.runCompiler(IncrementalJvmCompilerRunner.kt:102)
at org.jetbrains.kotlin.incremental.IncrementalCompilerRunner.compileIncrementally(IncrementalCompilerRunner.kt:225)
at org.jetbrains.kotlin.incremental.IncrementalCompilerRunner.access$compileIncrementally(IncrementalCompilerRunner.kt:39)
at org.jetbrains.kotlin.incremental.IncrementalCompilerRunner$compile$2.invoke(IncrementalCompilerRunner.kt:91)
at org.jetbrains.kotlin.incremental.IncrementalCompilerRunner.compile(IncrementalCompilerRunner.kt:103)
at org.jetbrains.kotlin.daemon.CompileServiceImpl.execIncrementalCompiler(CompileServiceImpl.kt:606)
at org.jetbrains.kotlin.daemon.CompileServiceImpl.access$execIncrementalCompiler(CompileServiceImpl.kt:102)
at org.jetbrains.kotlin.daemon.CompileServiceImpl$compile$$inlined$ifAlive$lambda$2.invoke(CompileServiceImpl.kt:455)
at org.jetbrains.kotlin.daemon.CompileServiceImpl$compile$$inlined$ifAlive$lambda$2.invoke(CompileServiceImpl.kt:102)
at org.jetbrains.kotlin.daemon.CompileServiceImpl$doCompile$$inlined$ifAlive$lambda$2.invoke(CompileServiceImpl.kt:1029)
at org.jetbrains.kotlin.daemon.CompileServiceImpl$doCompile$$inlined$ifAlive$lambda$2.invoke(CompileServiceImpl.kt:102)
at org.jetbrains.kotlin.daemon.common.DummyProfiler.withMeasure(PerfUtils.kt:137)
at org.jetbrains.kotlin.daemon.CompileServiceImpl.checkedCompile(CompileServiceImpl.kt:1071)
at org.jetbrains.kotlin.daemon.CompileServiceImpl.doCompile(CompileServiceImpl.kt:1028)
at org.jetbrains.kotlin.daemon.CompileServiceImpl.compile(CompileServiceImpl.kt:454)
at sun.reflect.GeneratedMethodAccessor103.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at sun.rmi.server.UnicastServerRef.dispatch(UnicastServerRef.java:357)
at sun.rmi.transport.Transport$1.run(Transport.java:200)
at sun.rmi.transport.Transport$1.run(Transport.java:197)
at java.security.AccessController.doPrivileged(Native Method)
at sun.rmi.transport.Transport.serviceCall(Transport.java:196)
at sun.rmi.transport.tcp.TCPTransport.handleMessages(TCPTransport.java:573)
at sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run0(TCPTransport.java:834)
at sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.lambda$run$0(TCPTransport.java:688)
at java.security.AccessController.doPrivileged(Native Method)
at sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run(TCPTransport.java:687)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.ClassCastException: org.jetbrains.kotlin.codegen.ErasedInlineClassBodyCodegen cannot be cast to org.jetbrains.kotlin.codegen.ImplementationBodyCodegen
at org.jetbrains.kotlin.codegen.ImplementationBodyCodegen.initializeObjects(ImplementationBodyCodegen.java:863)
at org.jetbrains.kotlin.codegen.ImplementationBodyCodegen.generateSyntheticPartsAfterBody(ImplementationBodyCodegen.java:410)
at org.jetbrains.kotlin.codegen.MemberCodegen.generate(MemberCodegen.java:130)
at org.jetbrains.kotlin.codegen.MemberCodegen.genClassOrObject(MemberCodegen.java:300)
at org.jetbrains.kotlin.codegen.MemberCodegen.genSyntheticClassOrObject(MemberCodegen.java:314)
at org.jetbrains.kotlin.codegen.ClassBodyCodegen.generateBody(ClassBodyCodegen.java:100)
at org.jetbrains.kotlin.codegen.MemberCodegen.generate(MemberCodegen.java:127)
at org.jetbrains.kotlin.codegen.ImplementationBodyCodegen.generateErasedInlineClassIfNeeded(ImplementationBodyCodegen.java:263)
at org.jetbrains.kotlin.codegen.ClassBodyCodegen.generateBody(ClassBodyCodegen.java:85)
at org.jetbrains.kotlin.codegen.MemberCodegen.generate(MemberCodegen.java:127)
at org.jetbrains.kotlin.codegen.MemberCodegen.genClassOrObject(MemberCodegen.java:300)
at org.jetbrains.kotlin.codegen.MemberCodegen.genClassOrObject(MemberCodegen.java:284)
at org.jetbrains.kotlin.codegen.PackageCodegenImpl.generateClassOrObject(PackageCodegenImpl.java:161)
at org.jetbrains.kotlin.codegen.PackageCodegenImpl.generateClassesAndObjectsInFile(PackageCodegenImpl.java:86)
at org.jetbrains.kotlin.codegen.PackageCodegenImpl.generateFile(PackageCodegenImpl.java:119)
at org.jetbrains.kotlin.codegen.PackageCodegenImpl.generate(PackageCodegenImpl.java:66)
... 43 more
Inline classes are not supported yet
+1 for this feature, inline classes will become more and more important
I'm not versed in the plugin's internal code, but wouldn't it be enough to just check for the body to be inlined (before this exception is thrown) and spawn a simple serializer wrapper around the internal value's serializer in that case?
inline class does not imply inlined serialization, the @n3phtys solution should be explicit flagged.
In a common use case a class, a data class and an inline class should preserve the binary compatibility.
My suggestion is to serialize the wrapper class as default behavior.
Finally considering the code:
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.JSON
fun main(args: Array<String>) {
println(JSON.stringify(Data.serializer(), Data("Works")))
println(JSON.stringify(Data.serializer(), Data("Bad resource", Error("Not found"))))
println(JSON.stringify(Data.serializer(), Data("Unknown error", Error(null))))
}
@Serializable
data class Data(val message: String, val error: Error? = null)
@Serializable
inline class Error(val description: String?)
The first and third example must be distinguishable.
I don't think this is an issue with inline, this seems to be a problem involving dealing with optional parameters.
Inline classes (for Kotlin) are syntactic sugar, until we have JVM value classes with a fixed and strong specification (which will take years IMO). Therefore the Kotlin compiler and everything plugin-y should always deal with them as they were nothing else. Under this assumption the first and third examples would be equivalent under serialization, but they're also indistinguishable from a bytecode signature. We only give them meaning by explicitly demanding a return type (in case of Java methods with similar type signature we use different names to distinguish two different methods).
IMO the output of the first and third example should be
{'message':'Works'}
Better yet would be to make the null explicit:
{'message':'Unknown error', 'error': null}
But AFAIK this is not what kotlinx.serialization does in general (instead it skips default parameters by default). So even though it hurts, the first output is the only logical one if the inline class Error should only be known at compile time.
Honestly I am happy any way, as long as we get OOTB implicit support, but we're actually not talking about a Kotlin type system, or even something as advanced as Haskell or similar languages. Serialization for us has to be based on an intersect between the JVM's and JSON's type system, so we will loose some 'type-truth' in any case.
Hi @n3phtys
I respect your point of view but I don't agree with it. I wish to comment some inaccuracy.
Inline classes (for Kotlin) are syntactic sugar
Everything in a programming language is "syntactic sugar".
until we have JVM value classes with a fixed and strong specification
Inline classes are language feature and work for all supported platforms, moreover JVM Q-Value and L-Value should encode in the same way.
this seems to be a problem involving dealing with optional parameters
None of the above parameters are @Optional.
Under this assumption the first and third examples would be equivalent under serialization
This is not my example, both message and error are required.
but they're also indistinguishable from a bytecode signature
This is false, please consult: https://github.com/Kotlin/KEEP/blob/master/proposals/inline-classes.md#inline-classes-over-nullable-types
Finally please consider future inline class evolution, multiple properties become reasonable for value classes.
@Serializable
inline class Error(val code: Int, val description: String?)
Should we expect inline classes support while it's still experimental?
@aviadmini We're trying to look into it
@aviadmini We're trying to look into it
Thanks in advance.
Currently unsigned types (eg. UInt) cannot be used in @Serializable classes because they are inline, too...
@fvasco Could the rule simply be: if and only if both the property with an inline class as its type and all the properties of the inline class are nullable, then the value is boxed? In all other cases there is no ambiguity. This rule would increase perf and make the wire format more readable in the vast majority of cases.
I really like inline classes but it's going to be annoying having to write so many custom serializers just to make the wire format efficient/understandable.
@DaanVandenBosch Yes, it could.
But I think that rule can result confusing for a human brain and it can become error prone.
I really like inline classes but it's going to be annoying having to write so many custom serializers just to make the wire format efficient/understandable.
Is it possible to reach this goal using a serializer's configuration, like an annotation?
Yes, an annotation would be good too. So by default the inline class would be boxed, with the annotation it would be inlined? Ideally it would be possible to add the annotation to either the inline class itself or the use-site of the inline class (i.e. a property with an inline class as type). If I had to choose one, I'd prefer an annotation on the inline class itself.
One of my use-cases for inline classes is an ID class per entity type. Using a different ID class per entity ensures you can't pass e.g. a ProductVariantID to a method or constructor that expects a ProductID. Sending big objects with lots of these IDs in boxed form can greatly increase their size. And in e.g. json, the output becomes hard to read.
@sandwwraith any thoughts on this from Jetbrains?
In our project we also have a huge amount of inline classes to improve type safety. We cannot accidentally assign wrong IDs as Daan has mentioned, we cannot accidentally assign an email address value to a phone number variable and so on.
Because these classes are inline we would expect it the JSON serialization to be inline too, i.e. inline class Box(val value: Int) serializes to value, not { "value": value }. With all the inline classes we've written so far, the latter is a very very rare case.
Our current JSON library serializes only the value by default, which has served us well so far :)
We've never used an inline class that wraps a nullable type (or that has a nullable upper bound) - in any project (I've just made a grep over all of them). No idea what would be a good approach here - but I guess it's an edge case, not the norm.
@DaanVandenBosch @fluidsonic For both your cases I would say that you would want to look at the issue as if it wasn't using inline classes. Basically this means you create a custom serializer. As the serializer code will be mainly identical you could use a shared base class for it.
@DaanVandenBosch @fluidsonic That's the right point – since the class is inline and have only one property, it is reasonable to serialize it as an underlying value, not as a JSON object. It should be true even if the property of the inline type itself is boxed (in JVM terms).
I can say that we have finished inline classes design in serialization and it's the next thing on a roadmap after sealed classes serialization, which has been released in 0.14.0.
I'm wondering how serializing inline classes will work for polymorphic serializers. In the current implementation of polymorphic serializers, if you have a polymorphic serializer for an interface where some or all of the implementations are inline classes the inline value would always be boxed when serializing (although not when deserializing) because polymorphic serializers use type checking to select the right serializer.
Right now it isn't really an issue because when using an inline class as an interface it implements it will be autoboxed anyway. However, if this gets implemented it seems like it would break polymorphic serialization.
Is this something you're thinking about in the current design or is this too speculative to take into consideration at this time?
@sandwwraith is there any place to checkout the progress about inline classes support?
@JavierSegoviaCordoba Here you go: https://youtrack.jetbrains.com/issue/KT-23338
This seems to be a bit more specific: https://youtrack.jetbrains.com/issue/KT-28791
Most helpful comment
@aviadmini We're trying to look into it