Preventing compilation of a @tailrec method when it does not rewrite, but an inner method does (#20143)

Adding warnings if there is an annotated def at the top level that is
referenced from an inner def

Fixes #20105

Author: sjrd

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -212,6 +212,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case ContextBoundCompanionNotValueID // errorNumber: 196
   case InlinedAnonClassWarningID // errorNumber: 197
   case UnusedSymbolID // errorNumber: 198
+  case TailrecNestedCallID //errorNumber: 199
 
   def errorNumber = ordinal - 1
 

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

@@ -1918,6 +1918,20 @@ class TailrecNotApplicable(symbol: Symbol)(using Context)
   def explain(using Context) = ""
 }
 
+class TailrecNestedCall(definition: Symbol, innerDef: Symbol)(using Context)
+  extends SyntaxMsg(TailrecNestedCallID) {
+  def msg(using Context) = {
+    s"The tail recursive def ${definition.name} contains a recursive call inside the non-inlined inner def ${innerDef.name}"
+  }
+
+  def explain(using Context) =
+    """Tail recursion is only validated and optimised directly in the definition.
+      |Any calls to the recursive method via an inner def cannot be validated as
+      |tail recursive, nor optimised if they are. To enable tail recursion from
+      |inner calls, mark the inner def as inline.
+      |""".stripMargin
+}
+
 class FailureToEliminateExistential(tp: Type, tp1: Type, tp2: Type, boundSyms: List[Symbol], classRoot: Symbol)(using Context)
   extends Message(FailureToEliminateExistentialID) {
   def kind = MessageKind.Compatibility

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

@@ -429,10 +429,23 @@ class TailRec extends MiniPhase {
           assert(false, "We should never have gotten inside a pattern")
           tree
 
-        case tree: ValOrDefDef =>
+        case tree: ValDef =>
           if (isMandatory) noTailTransform(tree.rhs)
           tree
 
+        case tree: DefDef =>
+          if (isMandatory)
+            if (tree.symbol.is(Synthetic))
+              noTailTransform(tree.rhs)
+            else
+              // We can't tail recurse through nested definitions, so don't want to propagate to child nodes
+              // We don't want to fail if there is a call that would recurse (as this would be a non self recurse), so don't
+              // want to call noTailTransform
+              // We can however warn in this case, as its likely in this situation that someone would expect a tail
+              // recursion optimization and enabling this to optimise would be a simple case of inlining the inner method
+              new NestedTailRecAlerter(method, tree.symbol).traverse(tree)
+          tree
+
         case _: Super | _: This | _: Literal | _: TypeTree | _: TypeDef | EmptyTree =>
           tree
 
@@ -446,14 +459,28 @@ class TailRec extends MiniPhase {
 
         case Return(expr, from) =>
           val fromSym = from.symbol
-          val inTailPosition = !fromSym.is(Label) || tailPositionLabeledSyms.contains(fromSym)
+          val inTailPosition = tailPositionLabeledSyms.contains(fromSym) // Label returns are only tail if the label is in tail position
+             || (fromSym eq method) // Method returns are only tail if we are looking at the original method
           cpy.Return(tree)(transform(expr, inTailPosition), from)
 
         case _ =>
           super.transform(tree)
       }
     }
   }
+
+  class NestedTailRecAlerter(method: Symbol, inner: Symbol) extends TreeTraverser {
+    override def traverse(tree: tpd.Tree)(using Context): Unit =
+      tree match {
+        case a: Apply =>
+          if (a.fun.symbol eq method) {
+            report.warning(new TailrecNestedCall(method, inner), a.srcPos)
+          }
+          traverseChildren(tree)
+        case _ =>
+          traverseChildren(tree)
+      }
+  }
 }
 
 object TailRec {

tests/neg/i20105.check

@@ -0,0 +1,10 @@
+-- [E199] Syntax Warning: tests/neg/i20105.scala:6:9 -------------------------------------------------------------------
+6 |      foo()
+  |      ^^^^^
+  |      The tail recursive def foo contains a recursive call inside the non-inlined inner def bar
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E097] Syntax Error: tests/neg/i20105.scala:3:4 ---------------------------------------------------------------------
+3 |def foo(): Unit = // error
+  |    ^
+  |    TailRec optimisation not applicable, method foo contains no recursive calls

tests/neg/i20105.scala

@@ -0,0 +1,9 @@
+import scala.annotation.tailrec
+@tailrec
+def foo(): Unit = // error
+  def bar(): Unit =
+    if (???)
+      foo()
+    else
+      bar()
+  bar()

tests/neg/i5397.scala

@@ -16,8 +16,10 @@ object Test {
         rec3 // error: not in tail position
     })
 
-  @tailrec def rec4: Unit = {
-    def local = rec4 // error: not in tail position
+  // This is technically not breaching tail recursion as rec4 does not call itself, local does
+  // This instead fails due to having no tail recursion at all
+  @tailrec def rec4: Unit = { // error: no recursive calls
+    def local = rec4
   }
 
   @tailrec def rec5: Int = {

tests/warn/i20105.check

@@ -0,0 +1,6 @@
+-- [E199] Syntax Warning: tests/warn/i20105.scala:6:9 ------------------------------------------------------------------
+6 |      foo() // warn
+  |      ^^^^^
+  |      The tail recursive def foo contains a recursive call inside the non-inlined inner def bar
+  |
+  | longer explanation available when compiling with `-explain`

tests/warn/i20105.scala

@@ -0,0 +1,10 @@
+import scala.annotation.tailrec
+@tailrec
+def foo(): Unit =
+  def bar(): Unit =
+    if (???)
+      foo() // warn
+    else
+      bar()
+  bar()
+  foo()