Skip to content

Spurious tailrec error "Cannot rewrite recursive call: it is not in tail position" because of asInstanceOf cast inserted by typer #8712

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

Closed
smarter opened this issue Apr 12, 2020 · 2 comments

Comments

@smarter
Copy link
Member

smarter commented Apr 12, 2020

Minimized code

import scala.annotation.tailrec

sealed trait Foo[+A]
case class Bar[A](f: Foo[A]) extends Foo[A]
case class Done[A](a: A) extends Foo[A]

object Test {
  @tailrec
  def runT[A](c: Foo[A]): A = c match {
    case Bar(f) => runT(f)
    case Done(a) => a
  }
}

Output

-- Error: try/tr.scala:10:23 ---------------------------------------------------
10 |    case Bar(f) => runT(f)
   |                   ^^^^^^^
   |               Cannot rewrite recursive call: it is not in tail position

The problem comes from the tree we get after erasure:

          if x9.isInstanceOf[Bar] then
            {
              case val x14: Bar = Bar.unapply(x9.asInstanceOf[Bar])
              case val x15: Foo = x14._1()
              case val f: Foo = x15
              return[matchResult7]
                {
                  Test.runT(f.asInstanceOf[Foo]).asInstanceOf[Object]
                }
            }
           else ()

runT is indeed not in tail position since it's followed by an asInstanceOf cast, but that shouldn't prevent tailrec from working. However, I don't know/remember enough about labelled blocks to be sure of the correct way to handle this.

@sjrd
Copy link
Member

sjrd commented Apr 12, 2020

It should prevent tailrec from working. An asInstanceOf is a real operation that requires something to happen.

However, in this case the asInstanceOf[Object] is completely useless. Obviously because everything is an Object, but in general expr.asInstanceOf[T] is useless if the type of expr is <: T according to the JVM type system.

Erasure is to blame here for inserting this useless cast. If it didn't insert that cast, tailrec would be able to do its job.

@smarter
Copy link
Member Author

smarter commented Apr 12, 2020

The cast is actually introduced by typer (I think because in general anything vaguely GADT-related gets a cast since GADT constraints are not known in phases after typer). But it could be removed by Erasure indeed. The cast-elision logic is:
https://github.com/lampepfl/dotty/blob/bd1fff257ca66156e555e782647588b4aad90365/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala#L243-L244
If I replace the check by erasure(expr.tpe) <:< testType the cast is elided as expected, but I haven't thought through whether that could break something else.

@smarter smarter changed the title Spurious tailrec error "Cannot rewrite recursive call: it is not in tail position" because of asInstanceOf cast inserted by erasure Spurious tailrec error "Cannot rewrite recursive call: it is not in tail position" because of asInstanceOf cast inserted by typer Apr 12, 2020
@smarter smarter self-assigned this Apr 12, 2020
smarter added a commit to dotty-staging/dotty that referenced this issue Apr 13, 2020
We can get rid of `asInstanceOf[T]` whenever the erased type of
expression is a subtype of T. This allows us to produce leaner bytecode
and also fixes scala#8712 since unnecessary asInstanceOf calls were
preventing tailrec from working.
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

2 participants