Kotlin / Kotlin/dataframe

Type inference incorrect w.r.t. variance

Open
#471 2 comments 0 reactions 1 assignee Claimed by @Jolanrensen View on GitHub
Dominant language
Kotlin
Stars
1.1k
Forks
83
Avg merge
4d 12h
Merged PRs (30d)
30

Description

There are some errors in the type inference of KTypes in DataFrame that make it behave differently compared to the Kotlin Compiler.

For instance, given
```kotlin
interface AbstractType
```
```kotlin
listOf>() + listOf>() // == List>
```
whereas DF would make it `List>`, ignoring the `out`-variance.

Similarly,
```kotlin
listOf>() + listOf>>() // == List>
```
whereas DF would make it `List>`, adding an `out`-variance which is not necessary.

DataFrame is the most incorrect is with `in`-variance:
```kotlin
listOf>() + listOf>() // == List>
```
yet, DF reports this as `List>`, which is just wrong.

This behavior is mostly regulated by [`Iterable.commonType()`](https://github.com/Kotlin/dataframe/blob/835de4b46c540a6504287c83a503cf7ad0a6fa83/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt#L141) and [`Iterable.commonTypeListifyValues()`](https://github.com/Kotlin/dataframe/blob/835de4b46c540a6504287c83a503cf7ad0a6fa83/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt#L226).

We need proper `KType` unification which takes variance into account when merging types respecting the [Kotlin Specification](https://kotlinlang.org/spec/pdf/kotlin-spec.pdf).
Specifically, given two `KTypeProjection`s, we need to be able to give the union/least upper bound/OR, and the intersection/greatest lower bound/AND. I don't know if there already exists a library that provides this logic, as understanding this part of the spec and recreating it is difficult to say the least.
The only similar place where this is done is actually inside the [Kotlin compiler itself](https://github.com/JetBrains/kotlin/blob/master/compiler/fir/providers/src/org/jetbrains/kotlin/fir/types/TypeUtils.kt) using `ConeKotlinType` and `ConeTypeProjection` instead of `KType` and `KTypeProjection`, but I'm not sure this can be reused.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.