From 5f4caae11c7a640350fbe90ddc465d8e2fa0156e Mon Sep 17 00:00:00 2001
From: Maybe Waffle <waffle.lapkin@gmail.com>
Date: Tue, 18 Jul 2023 16:25:59 +0000
Subject: [PATCH] Fix unconditional recursion lint wrt tail calls

---
 compiler/rustc_mir_build/src/lints.rs         | 18 ++++++++++++--
 ...lint-unconditional-recursion-tail-calls.rs | 24 +++++++++++++++++++
 ...-unconditional-recursion-tail-calls.stderr | 18 ++++++++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)
 create mode 100644 tests/ui/lint/lint-unconditional-recursion-tail-calls.rs
 create mode 100644 tests/ui/lint/lint-unconditional-recursion-tail-calls.stderr

diff --git a/compiler/rustc_mir_build/src/lints.rs b/compiler/rustc_mir_build/src/lints.rs
index 3cb83a48ffe..263e777d03a 100644
--- a/compiler/rustc_mir_build/src/lints.rs
+++ b/compiler/rustc_mir_build/src/lints.rs
@@ -196,8 +196,6 @@ fn node_examined(
             | TerminatorKind::CoroutineDrop
             | TerminatorKind::UnwindResume
             | TerminatorKind::Return
-            // FIXME(explicit_tail_calls) Is this right??
-            | TerminatorKind::TailCall { .. }
             | TerminatorKind::Unreachable
             | TerminatorKind::Yield { .. } => ControlFlow::Break(NonRecursive),
 
@@ -219,12 +217,28 @@ fn node_examined(
             | TerminatorKind::FalseUnwind { .. }
             | TerminatorKind::Goto { .. }
             | TerminatorKind::SwitchInt { .. } => ControlFlow::Continue(()),
+
+            // Note that tail call terminator technically returns to the caller,
+            // but for purposes of this lint it makes sense to count it as possibly recursive,
+            // since it's still a call.
+            //
+            // If this'll be repurposed for something else, this might need to be changed.
+            TerminatorKind::TailCall { .. } => ControlFlow::Continue(()),
         }
     }
 
     fn node_settled(&mut self, bb: BasicBlock) -> ControlFlow<Self::BreakVal> {
         // When we examine a node for the last time, remember it if it is a recursive call.
         let terminator = self.body[bb].terminator();
+
+        // FIXME(explicit_tail_calls): highlight tail calls as "recursive call site"
+        //
+        // We don't want to lint functions that recurse only through tail calls
+        // (such as `fn g() { become () }`), so just adding `| TailCall { ... }`
+        // here won't work.
+        //
+        // But at the same time we would like to highlight both calls in a function like
+        // `fn f() { if false { become f() } else { f() } }`, so we need to figure something out.
         if self.classifier.is_recursive_terminator(self.tcx, self.body, terminator) {
             self.reachable_recursive_calls.push(terminator.source_info.span);
         }
diff --git a/tests/ui/lint/lint-unconditional-recursion-tail-calls.rs b/tests/ui/lint/lint-unconditional-recursion-tail-calls.rs
new file mode 100644
index 00000000000..c94bf032579
--- /dev/null
+++ b/tests/ui/lint/lint-unconditional-recursion-tail-calls.rs
@@ -0,0 +1,24 @@
+#![allow(incomplete_features, dead_code)]
+#![deny(unconditional_recursion)] //~ note: the lint level is defined here
+#![feature(explicit_tail_calls)]
+
+fn f(x: bool) {
+    //~^ error: function cannot return without recursing
+    //~| note: cannot return without recursing
+    if x {
+        become f(!x)
+    } else {
+        f(!x) //~ note: recursive call site
+    }
+}
+
+// This should *not* lint, tail-recursive functions which never return is a reasonable thing
+fn g(x: bool) {
+    if x {
+        become g(!x)
+    } else {
+        become g(!x)
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/lint/lint-unconditional-recursion-tail-calls.stderr b/tests/ui/lint/lint-unconditional-recursion-tail-calls.stderr
new file mode 100644
index 00000000000..52f9740d027
--- /dev/null
+++ b/tests/ui/lint/lint-unconditional-recursion-tail-calls.stderr
@@ -0,0 +1,18 @@
+error: function cannot return without recursing
+  --> $DIR/lint-unconditional-recursion-tail-calls.rs:5:1
+   |
+LL | fn f(x: bool) {
+   | ^^^^^^^^^^^^^ cannot return without recursing
+...
+LL |         f(!x)
+   |         ----- recursive call site
+   |
+   = help: a `loop` may express intention better if this is on purpose
+note: the lint level is defined here
+  --> $DIR/lint-unconditional-recursion-tail-calls.rs:2:9
+   |
+LL | #![deny(unconditional_recursion)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+