Smart cast impossible in a coroutine context?


#1

Hi!

I’m using coroutines to load items from a data repository. Let’s imagine I’m talking about fetching pizzas.

The repository first checks a cache, which will hand back a (nullable) List<Pizza>?. Arguably it could instead return an emptyListOf<Pizza>() but that’s not the case here.) If that list is null, it instead fetches the pizzas from an API, which is guaranteed to return a (non-nullable) List<Pizza>. Here’s how I thought I’d accomplish just that:

class PizzasRepository {
  fun load(): Deferred<List<Pizza>> = async {
    val cache = loadFromCache()
    if (cache != null) {
      return@async cache
    }

    return@async lostFromApi().await()
  }

  private fun loadFromCache(): List<Pizza>? = cachedPizzas

  private fun loadFromApi(): List<Pizza> = async { listOf(Pizza()) }
}

However, this code doesn’t compile, with the following error:

Type inference failed. Expected type mismatch: inferred type is Deferred<List?> but Deferred<List> was expected

The problematic line is the return@async cache line, but I was under the impression that the compiler would be able to smart cast that to a List<Pizza>, because the val is checked above and is final.

Appending “bang bang” – with or without Jessie J, Ariana Grande & Nicki Minaj – the code compiles, but the linter tells me that the bangs are unnecessary:

fun load(): Deferred<List<Pizza>> = async {
  val cache = loadFromCache()

  if (cache != null) {
    // Unnecessary non-null assertion (!!) on a non-null receiver of type List<Pizza>
    return@async cache!!
  }

  return@async loadFromApi().await()
}

It might just be that I’m missing something crucial here, or there’s something not entirely right about coroutines and smart casts working together. Coroutines are, after all, in an experimental stage. :stuck_out_tongue_winking_eye:

Any thoughts?

Thanks!


#2

Yeah, looks like a compiler bug to me…

This seems to work around the bug:

fun load(): Deferred<List<Pizza>> = async {
    return@async loadFromCache() ?: loadFromApi().await()
}

Or you could also make it somewhat simpler, you can always make it Deferred at the call site by wrapping in async {} there:

class PizzasRepository {
    private var cachedPizzas: List<Pizza>? = null
    
    suspend fun load(): List<Pizza> {
        return loadFromCache() ?: loadFromApi().await()
    }

    private fun loadFromCache(): List<Pizza>? = cachedPizzas

    private fun loadFromApi(): Deferred<List<Pizza>> = async {
        listOf(Pizza()) 
    }
}

#3

Yeah, that’s what I figured as well. Your solution with our dearest Elvis would probably work, and for the time being I should be able to use that. Might become a less comprehensible once I add several levels of caching, but that’s an issue for another time. :smile:

Thanks a lot for your quick reply, @mslenc! You rock! :postal_horn:


#4

The work-around is to move type declaration in load function to the async builder like this:

fun load() = async<List<Pizza>> { ... }

I’ve created a ticket for this problem: https://youtrack.jetbrains.com/issue/KT-20817

As a matter of style, I highly recommend to include “async” into the name of functions that return Deferred<Something> or rather declare them as suspending functions and avoid the use of futures altogether, which makes the code easier to read/understand, faster, and less error-prone. I this case I’d rewrite it like this (no need to use async nor qualified returns at all):

class PizzasRepository {
    suspend fun load(): List<Pizza> =
        loadFromCache() ?: loadFromApi()

    private fun loadFromCache(): List<Pizza>? = TODO()
    private suspend fun loadFromApi(): List<Pizza> = TODO()
}

#5

Oh yeah, that is absolutely more readable, @elizarov! :+1:

And thanks for creating the YouTrack ticket. :smile: