Improve error reporting for Tailrec

Author: allanrenucci

compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala

@@ -1801,13 +1801,16 @@ object messages {
   case class TailrecNotApplicable(symbol: Symbol)(implicit ctx: Context)
     extends Message(TailrecNotApplicableID) {
     val kind: String = "Syntax"
-    val msg: String =
-      if (symbol.is(Method))
-        "TailRec optimisation not applicable, method not tail recursive"
-      else
-        hl"TailRec optimisation not applicable, ${symbol.showKind} isn't a method."
-    val explanation: String =
-      hl"A method annotated ${"@tailrec"} must be declared ${"private"} or ${"final"} so it can't be overridden."
+    val msg: String = {
+      val reason =
+        if (!symbol.is(Method)) hl"$symbol isn't a method"
+        else if (symbol.is(Deferred)) hl"$symbol is abstract"
+        else if (!symbol.isEffectivelyFinal) hl"$symbol is neither ${"private"} nor ${"final"} so can be overridden"
+        else hl"$symbol contains no recursive calls"
+
+      s"TailRec optimisation not applicable, $reason"
+    }
+    val explanation: String = ""
   }
 
   case class FailureToEliminateExistential(tp: Type, tp1: Type, tp2: Type, boundSyms: List[Symbol])(implicit ctx: Context)

compiler/src/dotty/tools/dotc/transform/TailRec.scala

@@ -116,12 +116,15 @@ class TailRec extends MiniPhase {
   override def transformDefDef(tree: DefDef)(implicit ctx: Context): Tree = {
     val method = tree.symbol
     val mandatory = method.hasAnnotation(defn.TailrecAnnot)
-    def noTailTransform = {
+    def noTailTransform(failureReported: Boolean) = {
       // FIXME: want to report this error on `tree.namePos`, but
       // because of extension method getting a weird pos, it is
-      // better to report on method symbol so there's no overlap
-      if (mandatory)
+      // better to report on method symbol so there's no overlap.
+      // We don't report a new error if failures were reported
+      // during the transformation.
+      if (mandatory && !failureReported)
         ctx.error(TailrecNotApplicable(method), method.pos)
+
       tree
     }
 
@@ -182,14 +185,15 @@ class TailRec extends MiniPhase {
           )
         )
       }
-      else noTailTransform
+      else noTailTransform(failureReported = transformer.failureReported)
     }
-    else noTailTransform
+    else noTailTransform(failureReported = false)
   }
 
   class TailRecElimination(method: Symbol, enclosingClass: ClassSymbol, paramSyms: List[Symbol], isMandatory: Boolean) extends TreeMap {
 
     var rewrote: Boolean = false
+    var failureReported: Boolean = false
 
     /** The `tailLabelN` label symbol, used to encode a `continue` from the infinite `while` loop. */
     private[this] var myContinueLabel: Symbol = _
@@ -276,8 +280,12 @@ class TailRec extends MiniPhase {
           cpy.Apply(tree)(noTailTransform(tree.fun), arguments)
 
         def fail(reason: String) = {
-          if (isMandatory) ctx.error(s"Cannot rewrite recursive call: $reason", tree.pos)
-          else tailrec.println("Cannot rewrite recursive call at: " + tree.pos + " because: " + reason)
+          if (isMandatory) {
+            failureReported = true
+            ctx.error(s"Cannot rewrite recursive call: $reason", tree.pos)
+          }
+          else
+            tailrec.println("Cannot rewrite recursive call at: " + tree.pos + " because: " + reason)
           continue
         }
 
@@ -339,7 +347,7 @@ class TailRec extends MiniPhase {
           else fail("it is not in tail position")
         }
         else if (isRecursiveSuperCall)
-          fail("it contains a recursive call targeting a supertype")
+          fail("it targets a supertype")
         else
           continue
       }

tests/neg-tailcall/t1672b.scala

@@ -2,7 +2,7 @@ import annotation.tailrec
 
 object Test1772B {
   @tailrec
-  def bar : Nothing = { // error: TailRec optimisation not applicable
+  def bar : Nothing = {
     try {
       throw new RuntimeException
     } catch {
@@ -13,7 +13,7 @@ object Test1772B {
   }
 
   @tailrec
-  def baz : Nothing = { // error: TailRec optimisation not applicable
+  def baz : Nothing = {
     try {
       throw new RuntimeException
     } catch {
@@ -24,7 +24,7 @@ object Test1772B {
   }
 
   @tailrec
-  def boz : Nothing = { // error: TailRec optimisation not applicable
+  def boz : Nothing = {
     try {
       throw new RuntimeException
     } catch {
@@ -33,7 +33,7 @@ object Test1772B {
   }
 
   @tailrec
-  def bez : Nothing = { // error: TailRec optimisation not applicable
+  def bez : Nothing = {
     try {
       bez // error: it is not in tail position
     } finally {

tests/neg-tailcall/t6574.scala

@@ -1,7 +1,7 @@
 import annotation.tailrec
 
 class Bad[X, Y](val v: Int) extends AnyVal {
-  @tailrec final def notTailPos[Z](a: Int)(b: String): Unit = { // error: TailRec optimisation not applicable
+  @tailrec final def notTailPos[Z](a: Int)(b: String): Unit = {
     this.notTailPos[Z](a)(b) // error: it is not in tail position
     println("tail")
   }

tests/neg-tailcall/tailrec-2.scala

@@ -7,7 +7,7 @@ sealed abstract class Super[+A] {
 // This one should fail, target is a supertype
 class Bop1[+A](val element: A) extends Super[A] {
 
-  @tailrec final def f[B >: A](mem: List[B]): List[B] = // error: TailRec optimisation not applicable
+  @tailrec final def f[B >: A](mem: List[B]): List[B] =
     (null: Super[A]).f(mem) // error: recursive call targeting a supertype
 
   @tailrec final def f1[B >: A](mem: List[B]): List[B] = this.g(mem) // error: TailRec optimisation not applicable

tests/neg-tailcall/tailrec.scala

@@ -47,18 +47,17 @@ class Winners {
 
 object Failures {
   @tailrec
-  def facfail(n: Int): Int =                          // error
+  def facfail(n: Int): Int =
     if (n == 0) 1
-    else n * facfail(n - 1)                           // error
+    else n * facfail(n - 1) // error: not in tail pos
 }
 
 class Failures {
-  // not private, not final
-  @tailrec def fail1(x: Int): Int = fail1(x)          // error
+  @tailrec def fail1(x: Int): Int = fail1(x) // error: not private, not final
 
   // a typical between-chair-and-keyboard error
-  @tailrec final def fail2[T](xs: List[T]): List[T] = xs match {   // error
+  @tailrec final def fail2[T](xs: List[T]): List[T] = xs match {
     case Nil      => Nil
-    case x :: xs  => x :: fail2[T](xs)                // error
+    case x :: xs  => x :: fail2[T](xs) // error: not in tail pos
   }
 }

tests/neg-tailcall/while-loops.scala

@@ -3,13 +3,13 @@ import annotation.tailrec
 object WhileLoops {
   def cond: Boolean = ???
 
-  @tailrec def rec1: Unit = { // error: tailrec not applicable
+  @tailrec def rec1: Unit = {
     while (cond) {
       rec1 // error: not in tail position
     }
   }
 
-  @tailrec def rec2: Boolean = { // error: tailrec not applicable
+  @tailrec def rec2: Boolean = {
     while (rec2) { } // error: not in tail position
     true
   }

tests/neg/i4196.scala

@@ -1,6 +1,6 @@
 object Test {
   @annotation.tailrec
-  def foo(i: implicit Unit => Int): implicit Unit => Int =  // error: method not tail recursive
+  def foo(i: implicit Unit => Int): implicit Unit => Int =
     if (i == 0)
       0
     else