Remove remaining unsafe cast from Resource interpreters

[bplommer]

Somewhat amazingly, it turns out we don't even need the final cast when outputting the result (though IntelliJ complains).

[djspiewak]

Incredible

[milessabin]

While I'm 💯 % 👍 for this change, I have a hunch that on 2.x it's only compiling by accident (ie. it would continue to compile even if the RHS of the pattern was unsound). I've run into these kinds of false dawn before, and the code here is reminiscent of some of the examples here.

That said, Dotty is getting this right now, so even if scalac is giving us a false sense of security here, the fact that it cross compiles with dotc should give us confidence that we're not getting away with anything we're not genuinely entitled to.

[smarter]

Dotty does warn about some stuff here so no guarantee of soundness:

 [warn] -- Warning: /home/runner/work/cats-effect/cats-effect/core/shared/src/main/scala/cats/effect/Resource.scala:119:13 
[warn] 119 |        case a: Allocate[G, C] =>
[warn]     |             ^^^^^^^^^^^^^^^^^
[warn]     |the type test for cats.effect.Resource.Allocate[G, C] cannot be checked at runtime

[smarter]

(these all come from the fact that Resource is covariant in its type parameters but its subclasses aren't)

[bplommer]

I have a hunch that on 2.x it's only compiling by accident (ie. it would continue to compile even if the RHS of the pattern was unsound).

It no longer compiles if you try to call the onOutput(a) from the Frame branch of the pattern match. It's also possible to summon an instance of C <:< A from the Nil branch, but not from the Frame branch. So that makes me think it's not an accident, no?

[SystemFw]

Incredible

This is the appropriate reaction 😄

[bplommer]

(these all come from the fact that Resource is covariant in its type parameters but its subclasses aren't)

That can be fixed in Dotty by using _ for all the type parameters in the match, but Scala 2.x can't infer the type constructor if you do that 😞

[milessabin]

Shouldn't the subclasses also be covariant?

[smarter]

That can be fixed in Dotty by using _ for all the type parameters in the match, but Scala 2.x can't infer the type constructor if you do that disappointed

Ah, and if if you do that you don't need asInstanceOf? That sounds good then

Shouldn't the subclasses also be covariant?

Most of them can't be due to the positions in which their parameters are used, there was some related discussion on gitter a few weeks ago.

[bplommer]

That can be fixed in Dotty by using _ for all the type parameters in the match, but Scala 2.x can't infer the type constructor if you do that disappointed

Ah, and if if you do that you don't need asInstanceOf? That sounds good then

Correct, though interestingly Dotty then fails to find the flatMap extension method on line 133, so you need the desugared version.