scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
230 stars 21 forks source link

Missing @tailrec error with recursive call targeting supertype #11442

Open smarter opened 5 years ago

smarter commented 5 years ago

The following outputs an error as expected:

import scala.annotation.tailrec

trait Outer { self =>
  def hi(cond: Boolean): Boolean

  trait Inner extends Outer {
    @tailrec
    final def hi(cond: Boolean): Boolean =
      self.hi(cond)
  }
}
try/super.scala:9: error: could not optimize @tailrec annotated method hi: it contains a recursive call targeting a supertype
      self.hi(cond)
           ^
one error found

The following also outputs an error:

import scala.annotation.tailrec

trait Outer { self =>
  def hi(cond: Boolean): Boolean

  trait Inner extends Outer {
    @tailrec
    final def hi(cond: Boolean): Boolean =
      self.hi(cond) && cond
  }
}
try/super.scala:8: error: could not optimize @tailrec annotated method hi: it contains a recursive call not in tail position
    final def hi(cond: Boolean): Boolean =
              ^
one error found

But this compiles without error:

import scala.annotation.tailrec

trait Outer { self =>
  def hi(cond: Boolean): Boolean

  trait Inner extends Outer {
    @tailrec
    final def hi(cond: Boolean): Boolean =
      self.hi(cond) && hi(cond)
  }
}
Jasper-M commented 3 months ago

I think it's open to interpretation whether Scala 2 or 3 is right here.

For instance, this example compiles in both 2 and 3:

import scala.annotation.tailrec

trait HiHo {
  final def ho(cond: Boolean): Boolean = hi(cond)
  @tailrec
  final def hi(cond: Boolean): Boolean =
    ho(cond) && hi(cond)
}

Essentially it's the same thing: there are 2 branches, one of which can be tailrec optimized and one which cannot. And actually this example can be proven to eventually stackoverflow while self.hi(cond) could be perfectly safe.

joroKr21 commented 3 months ago

I don't think @tailrec can handle mutual recursion

Jasper-M commented 3 months ago

Indeed it can't, but basically my point is that scala 2 accepts @tailrec as long as there is a self-recursive call that can be optimized. Whether or not there are other method calls in there doesn't matter. Scala 3 does the same thing except when one of those other method calls looks like a recursive call but targets a supertype.

In both cases the compiler manages to optimize an actual tail recursive call. And in both cases that other method call is not guaranteed to be either safe or unsafe.

joroKr21 commented 3 months ago

Ahh I see what you mean, that makes sense 👍

som-snytt commented 3 months ago

Not sure what the context is (that motivated commenting), but this is one of a few tailrec tickets I'm addressing (today or so). There is also a ticket on mutual recursion of local methods. Hopefully after coffee and sunrise, the comments will also make sense to me.

joroKr21 commented 3 months ago

We don't really know if the super method is recursive or not.

som-snytt commented 3 months ago

The preference on the dotty ticket for x => f() was to warn conservatively for any possibility of recursivity.