From adf12f380a5e7c3928d61c923351dbbd2f8f8614 Mon Sep 17 00:00:00 2001
From: "Zack M. Davis" <code@zackmdavis.net>
Date: Mon, 11 Jan 2021 10:24:05 -0800
Subject: [PATCH] don't gratuitously error on tests returning Result with
 lifetime
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Sander Maijers reports that #[test] functions that return a `Result`
with a named lifetime fail to compile with a "functions used as tests
must have signature fn() -> ()" error message—but that can't really be
right, because #[test] functions that return a `Result` with a static
lifetime are accepted!

It turns out that this error message dates all the way back to April
2013's 5f1a90ebe ("Issue 4391: rustc should not silently skip tests
with erroneous signature."). But after RFC 1937, we actually do accept
non-unit return types in tests. Let's edit the error message
accordingly.

This resolves #55228, and is of historical interest to #4391.
---
 compiler/rustc_ast/src/ast.rs             |  6 +++
 compiler/rustc_builtin_macros/src/test.rs |  4 +-
 src/test/ui/test-fn-with-lifetime.rs      | 47 +++++++++++++++++++++++
 src/test/ui/test-fn-with-lifetime.stderr  | 21 ++++++++++
 4 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 src/test/ui/test-fn-with-lifetime.rs
 create mode 100644 src/test/ui/test-fn-with-lifetime.stderr

diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs
index ff89982a4efd8..dd57d7acc9601 100644
--- a/compiler/rustc_ast/src/ast.rs
+++ b/compiler/rustc_ast/src/ast.rs
@@ -392,6 +392,12 @@ pub struct GenericParam {
     pub kind: GenericParamKind,
 }
 
+impl GenericParam {
+    pub fn is_lifetime(&self) -> bool {
+        matches!(self.kind, GenericParamKind::Lifetime)
+    }
+}
+
 /// Represents lifetime, type and const parameters attached to a declaration of
 /// a function, enum, trait, etc.
 #[derive(Clone, Encodable, Decodable, Debug)]
diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs
index 25d3f46da6cdc..7b1614e637651 100644
--- a/compiler/rustc_builtin_macros/src/test.rs
+++ b/compiler/rustc_builtin_macros/src/test.rs
@@ -458,8 +458,8 @@ fn has_test_signature(cx: &ExtCtxt<'_>, i: &ast::Item) -> bool {
                 false
             }
             (true, false) => {
-                if !generics.params.is_empty() {
-                    sd.span_err(i.span, "functions used as tests must have signature fn() -> ()");
+                if !generics.params.iter().all(|p| p.is_lifetime()) {
+                    sd.span_err(i.span, "return value of functions used as tests must either be `()` or implement `Termination`, and cannot use generics");
                     false
                 } else {
                     true
diff --git a/src/test/ui/test-fn-with-lifetime.rs b/src/test/ui/test-fn-with-lifetime.rs
new file mode 100644
index 0000000000000..f03ea0ed16018
--- /dev/null
+++ b/src/test/ui/test-fn-with-lifetime.rs
@@ -0,0 +1,47 @@
+// compile-flags: --test
+
+// Issue #55228
+
+#![feature(termination_trait_lib)]
+
+use std::process::Termination;
+
+#[test]
+fn unit_1() -> Result<(), &'static str> { // this is OK
+    Ok(())
+}
+
+#[test]
+fn unit_2<'a>() -> Result<(), &'a str> { // also OK
+    Ok(())
+}
+
+#[test]
+fn unit_3<T: Termination>() -> Result<(), T> { // nope (how would this get monomorphized?)
+    //~^ ERROR return value of functions used as tests must either be `()` or implement
+    Ok(())
+}
+
+
+struct Quux;
+
+trait Bar {
+    type Baz;
+
+    fn rah(&self) -> Self::Baz;
+}
+
+impl Bar for Quux {
+    type Baz = String;
+
+    fn rah(&self) -> Self::Baz {
+        "rah".to_owned()
+    }
+}
+
+#[test]
+fn unit_4<T: Bar<Baz = String>>() -> <T as Bar>::Baz { // also nope
+    //~^ ERROR return value of functions used as tests must either be `()` or implement
+    let q = Quux{};
+    <Quux as Bar>::rah(&q)
+}
diff --git a/src/test/ui/test-fn-with-lifetime.stderr b/src/test/ui/test-fn-with-lifetime.stderr
new file mode 100644
index 0000000000000..1e19b6b0a96c1
--- /dev/null
+++ b/src/test/ui/test-fn-with-lifetime.stderr
@@ -0,0 +1,21 @@
+error: return value of functions used as tests must either be `()` or implement `Termination`, and cannot use generics
+  --> $DIR/test-fn-with-lifetime.rs:20:1
+   |
+LL | / fn unit_3<T: Termination>() -> Result<(), T> { // nope (how would this get monomorphized?)
+LL | |
+LL | |     Ok(())
+LL | | }
+   | |_^
+
+error: return value of functions used as tests must either be `()` or implement `Termination`, and cannot use generics
+  --> $DIR/test-fn-with-lifetime.rs:43:1
+   |
+LL | / fn unit_4<T: Bar<Baz = String>>() -> <T as Bar>::Baz { // also nope
+LL | |
+LL | |     let q = Quux{};
+LL | |     <Quux as Bar>::rah(&q)
+LL | | }
+   | |_^
+
+error: aborting due to 2 previous errors
+