Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@tailrec false positive #21642

Open
sir-wabbit opened this issue Sep 24, 2024 · 3 comments
Open

@tailrec false positive #21642

sir-wabbit opened this issue Sep 24, 2024 · 3 comments

Comments

@sir-wabbit
Copy link

sir-wabbit commented Sep 24, 2024

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 sir-wabbit added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Sep 24, 2024
@sir-wabbit
Copy link
Author

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
Copy link
Author

sir-wabbit commented Sep 24, 2024

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
Copy link
Contributor

som-snytt commented Sep 24, 2024

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.

@Gedochao Gedochao added area:annotations and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Sep 26, 2024
@Gedochao Gedochao changed the title @tailrec false negative @tailrec false positive Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants