scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.85k stars 1.05k forks source link

@tailrec false positive #21642

Open sir-wabbit opened 3 weeks ago

sir-wabbit commented 3 weeks ago

Compiler version

3.5.0

Minimized code

EDIT: found a way to minimize the example:

@tailrec def minimal9_2[A](value: Option[Int]): A = {
  value match {
    case Some(n) => minimal9_2(None)
    case _ => ???
  }
}

Original snippet:

import scala.annotation.tailrec

final class Need[+A] private (private var thunk: Any) {
  def value: A = ???
}

object Need {
  type ThunkD[+A] = AnyRef
  final case class BindL[A](node: Need[A], run: Any => Need[A]) extends ThunkD[A]
  type BindR[A] = Need[A] // <: ThunkD[A]

  import java.util.{ArrayList => Q}
  final class BufRef(var buf: Q[ThunkD[Any]] = null)

  @tailrec def evalSlow[A, Z](ref: BufRef, count: Int, current: Need[A]): Z = {
    val r = current.thunk
    val value: ThunkD[Any] = ???
    value match {
      case BindL(c, f) =>
        val next = f(r)
        c.thunk = next.thunk
        ref.buf.add(c: BindR[Any])
        evalSlow(ref, count, next)
      case c =>
        val c1 = c.asInstanceOf[BindR[_]]
        c1.thunk = r
        evalSlow(ref, count - 1, c1)
    }
  }
}

Output

Cannot rewrite recursive call: it is not in tail position

on both evalSlow calls.

Expectation

Should work since they are in tail position, works in Scala 2.12.

sir-wabbit commented 3 weeks ago

Found a way to minimize it further (I think this is the same bug):

@tailrec def minimal9[A](count: Int, value: Either[Int, String]): A = {
  value match {
    case Left(n) if n > 0 => minimal9(count, Right(n.toString))
    case Right(s) => minimal9(count - 1, Left(s.length))
    case _ => minimal9(count, Left(0))
  }
}

Note that removing type parameter makes the issue go away.

sir-wabbit commented 3 weeks ago

Even further minimized:

@tailrec def minimal9_1[A](value: Option[Int]): A = {
  value match {
    case Some(n) if n > 0 => minimal9_1(Some(n + 1))
    case _ => ???
  }
}
@tailrec def minimal9_2[A](value: Option[Int]): A = {
  value match {
    case Some(n) => minimal9_2(None)
    case _ => ???
  }
}
som-snytt commented 3 weeks ago

I think this is a "false positive" because the test result says there is a problem. A health screening test that comes back negative is a good thing, unless it is a false negative. ("Dodgers optimistic for Freeman's playoff status as ankle X-rays negative".)

Workaround is to use an explicit type arg (which is otherwise inferred to be Nothing):

minimal9_2[A](None)

Edit: or

??? : A

IIUC it's not supposed to be necessary due to erasure:

https://github.com/scala/scala3/blob/main/compiler/src/dotty/tools/dotc/transform/TailRec.scala#L98-L100

I don't know why the comment is the opposite of reality.