ClassCastException in translate #1070
Comments
|
not sure if translate`s Uncons case is correctly defined |
|
The bug is definitely in case un: Uncons[F, x, O2] =>
val x: FreeC[Algebra[G, x, ?], Unit] = FreeC.suspend(un.s.translate(algFtoG))
val y: Algebra[G, O2, Option[(Segment[x, Unit], FreeC[Algebra[G, x, ?], Unit])]] =
Uncons[G, x, O2](x, un.chunkSize, un.maxSteps)
val res: Algebra[G, O2, X] = y
resThe assignment of |
|
I don't know though -- |
|
is this the only case in which |
|
@SystemFw Great question -- some cases work fine, so maybe the unsoundness argument is flawed. scala> scodec.stream.decode.tryMany(scodec.codecs.int32).decode[IO](hex"00112233445566778899".bits).compile.toVector.unsafeRunSync
res8: Vector[Int] = Vector(1122867, 1146447479) |
|
well, I guess the next step would be minimisation. I'm still slightly out of my depth on the core interpreter unfortunately 😞 (can't wait for it to stabilise so I can deep dive) |
|
Mm another question is whether we can take |
|
Here's a reproduction without scodec: "translate (2)" in forAll { (s: PureStream[Int]) =>
runLog(
s.get
.covary[Function0]
.flatMap(i => Stream.eval(() => i))
.flatMap(i => Stream.eval(() => i))
.translate(new (Function0 ~> IO) {
def apply[A](thunk: Function0[A]) = IO(thunk())
})
) shouldBe runLog(s.get)
}Results in: |
|
If the second |
|
If you elide the first |
|
Right -- in order to get the class cast exception, you need an |
|
I've lost track of whether having |
|
Me too -- hoping @pchlupacek has some ideas. :) |
|
@mpilquist @SystemFw excellent you put up minimalized case. I belive indeed the issue is related to NOT transalting the F ~> G in tail poisition after uncons is made. The reason why Uncons went to algebra is to solve SoE (#1035). Later it turned out that it allows for much more reliable interruption implementation. I have some ideas on fixing this. I will explore them today, and let you know the progress. |
|
@mpilquist @SystemFw I have managed to define translate with FreeC flatMap and get rid of effect instance from the algebra (bugfix/translate-typecast). As such, there are no longer any typecasts in |
|
Also this is the minimal stream where this fails: |
|
one approach I am thinking of now is to define uncons as final case class Uncons[F[_], X, O](
s: FreeC[Algebra[F, X, ?], Unit],
f: Option[(Segment[X, Unit], FreeC[Algebra[F, X, ?], Unit])] => F[FreeC[Algebra[F, O, ?], Unit])],
chunkSize: Int,
maxSteps: Long)But not sure if that is possible |
|
of course that will lead to definition of uncons like def uncons[O2, R](f: Option[(Segment[O, Unit], Stream[F, O])] => Pull[F,O2,R]): Pull[F, O2, R] = ???
which is quite a significant change of the API. |
|
OTOH perhaps deffinition of def uncons[F2, O2](f: Option[(Segment[O, Unit], Stream[F, O])] => Stream[F2, O2]]): Stream[F2, O2]May allow to get rid of Pull alltogether :-) |
|
well, I think the advantage of having both Pull and Stream is that having a Monad instance on the return type (Pull) makes it easier to write stateful combinators (return the rest), whereas having it on the output type (Stream) is better for program flow/FRP. In fact this is an advantage that even haskell libraries forego |
|
These are very rough ideas. I think the root of the cause is that Uncons are really fused two operations into one, I mean Uncons itself and F ~> G, given F is Algebra[F, X, ? ], G is Algebra[F, O, ?]. Would be interesting if we can sort of make it two independent operations, in that case, we may perhaps define translate safely. I have guess that root of the cause why translate doesn't work is that the |
|
Also we may still have Pull, available but would be just type alias something like |
|
which is sort of today anyhow. |
|
I really don’t want to change the Pull API at this point (and it is important for pulls to be able to emit values) |
|
yes, completely understand just bringing ideas on the table :-) |
|
@mpilquist Fixed in #1073. Could you perhaps try again to run that branch against the scodec tests? Thanks. |
|
Awesome! |
|
ah excellent:-) |
|
that's great! :) |
|
@ChristopherDavenport tried it on doobie and the tests pass. |
|
closed by #1073 |
Tried building scodec-stream against 0.10.0-RC1 and encountered a
ClassCastException:The text was updated successfully, but these errors were encountered: