Kotlin bugs with value classes

[nikclayton]

This is more of an FYI than a bug report for kotlin-result, although you might want to highlight this in the README.

I see with 2.0 you've switched to an inline value class. This will (I think) cause problems using kotlin-result with Retrofit, Ktor, SQLDelight (other libraries may also be affected, these are just the ones I'm aware of).

See for example, these issues which relate to the Kotlin stdlib.Result with those libraries, where the underlying cause seems to be that stdlib.Result is an inline value class.

• java.lang.ClassCastException: class kotlin.Result cannot be cast to class java.lang.String (kotlin.Result is in unnamed module of loader 'app'; java.lang.String is in module java.base of loader 'bootstrap') Kotlin/kotlinx.coroutines#3403
• https://youtrack.jetbrains.com/issue/KT-53559/JVM-ClassCastException-class-kotlin.Result-cannot-be-cast-to-class-java.lang.String-with-Retrofit
• https://youtrack.jetbrains.com/issue/KT-27105
• https://youtrack.jetbrains.com/issue/KT-27586
• Using inline class for id results in ClassCastException at runtime sqldelight/sqldelight#1285

[michaelbull]

Thanks for the report. I'd put off adopting inline value classes for many years until they had been marked as stable, which they were in Kotlin 1.5 as per the YouTrack for them, which was released 3 years ago.

If there's a workaround consumers need to do to solve problems with the Kotlin compiler, then I'll consider adding them to the README provided we can link to a report on the Kotlin compiler bug itself - however it seems like most of the actual kotlinc issues are now marked as resolved.

I don't really wish to start listing problems in the README of this project that other libraries are causing by not complying with a stable language feature - that list could theoretically be endless and paints this project in a negative light when it isn't responsible for those problems.

[nikclayton]

Unfortunately I'm not sure there's a workaround that library developers can use (if they use reflection anywhere).

AIUI, and to use Retrofit as an example, in something like this (using kotlin.stdlib.Result, but I think the problem will be the same with this library), consider this Retrofit API interface definition:

interface SomeApi {
    @GET("/some/route")
    suspend fun someRoute(): Result<String>
}

Retrofit will use the interface definition and runtime reflection to create a proxy class, inspecting the someRoute method's signature to create the implementation in the proxy.

However, due to the way functions that use inline classes are mangled to avoid platform signature clashes, the signature of the generated function will (or may, depending on the compiler's choice) be fun someRoute(): String, but the body of the function (generated by Retrofit) will try and return a Result<String>.

And then you get a runtime error like:

java.lang.ClassCastException: kotlin.Result cannot be cast to String

I think this is the heart of the problem in https://youtrack.jetbrains.com/issue/KT-53559/JVM-ClassCastException-class-kotlin.Result-cannot-be-cast-to-class-java.lang.String-with-Retrofit, which is the YT issue that is still open and still affects Kotlin 1.9.22 and 2.0.0-Beta4.

I don't think there's anything Retrofit or other libraries that use reflection can do here. Whether or not the value inside the value class is boxed/unboxed is something that happens at the whim of the compiler, and Retrofit couldn't detect the name mangling, or derive the original return type from the mangled method name.

In addition to these, an value class is allowed by the implementation to be inlined where applicable, so that its data property is operated on instead. This also means that the property may be boxed back to the value class by using its primary constructor at any time if the compiler decides it is the right thing to do.
- The Kotlin spec (https://kotlinlang.org/spec/pdf/kotlin-spec.pdf, 1.9-rfc+0.1), sect. 4.1.5, emphasis mine

[michaelbull]

Retrofit's problem is being tracked alread on their own pull request that adds an adapter for the kotlin.Result type. I would anticipate consumers will need to implement their own adapter for any other inline value classes (such as this one) unless they try to tackle all of them broadly at once - it seems as though they are just targeting the kotlin.Result type explicitly here though.

[nikclayton]

Retrofit's problem is being tracked alread on their square/retrofit#4018 (comment) that adds an adapter for the kotlin.Result type

Some history might be helpful.

This was already tried with https://github.com/connyduck/calladapter, which provided a Retrofit adapter for stdlib.Result that used an inline class.

That had to be deprecated by its author and replaced with https://github.com/connyduck/networkresult-calladapter which uses a custom, non-inline-class Result type, precisely because of this problem with inline classes.

The author of square/retrofit#4018 is trying to do essentially the same thing that https://github.com/connyduck/calladapter did, and is discovering, through comments on their PR, that it won't work. Worse, it will appear to work some of the time, and then blow up unpredictably at run time.

I've already written an adapter for kotlin-result 1.x (https://github.com/pachli/pachli-android/tree/main/core/network/src/main/kotlin/app/pachli/core/network/retrofit/apiresult) because I agree with your design decision that the failure type should not need to be an Exception or Throwable. And the overall API feels better.

But I don't think it's possible to write a normal adapter for kotlin-result 2.x, because it's normally impossible to know whether the property has been boxed by the compiler.

There are some exceptions; for example, kotlinx.serialization works with inline classes, but only because they also provide a compiler plugin so they have the full type information. Ksp might also work, with androidx/androidx@446445c. Both of those obviously require additional work at build time, and additional development work.

I appreciate that none of this is your problem to solve, and I don't expect you to. But I do think a notice for the 2.x series, something like "kotlin-result is implemented using inline classes, which are known to have problems when used with libraries that use reflection, like Retrofit" (perhaps with a link to this issue in your repository, and to https://youtrack.jetbrains.com/issue/KT-53559/JVM-ClassCastException-class-kotlin.Result-cannot-be-cast-to-class-java.lang.String-with-Retrofit) would be helpful to people when making a decision about whether or not to adopt kotlin-result.