Auto merge of #115538 - lcnr:fn-def-wf, r=compiler-errors

check `FnDef` return type for WF

better version of #106807, fixes #84533 (mostly). It's not perfect given that we still ignore WF requirements involving bound regions but I wasn't able to quickly write an example, so even if theoretically exploitable, it should be far harder to trigger.

This is strictly more restrictive than checking the return type for WF as part of the builtin `FnDef: FnOnce` impl (#106807) and moving to this approach in the future will not break any code.

~~It also agrees with my theoretical view of how this should behave~~

r? types

8 files changed, 153 insertions, 45 deletions

diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs
index 7941a8fe95c..19ca147d3ad 100644
--- a/compiler/rustc_trait_selection/src/traits/wf.rs
+++ b/compiler/rustc_trait_selection/src/traits/wf.rs
@@ -647,6 +647,8 @@ impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> {
     fn visit_ty(&mut self, t: <TyCtxt<'tcx> as ty::Interner>::Ty) -> Self::Result {
         debug!("wf bounds for t={:?} t.kind={:#?}", t, t.kind());
 
+        let tcx = self.tcx();
+
         match *t.kind() {
             ty::Bool
             | ty::Char
@@ -707,6 +709,16 @@ impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> {
             }
 
             ty::FnDef(did, args) => {
+                // HACK: Check the return type of function definitions for
+                // well-formedness to mostly fix #84533. This is still not
+                // perfect and there may be ways to abuse the fact that we
+                // ignore requirements with escaping bound vars. That's a
+                // more general issue however.
+                //
+                // FIXME(eddyb) add the type to `walker` instead of recursing.
+                let fn_sig = tcx.fn_sig(did).instantiate(tcx, args);
+                fn_sig.output().skip_binder().visit_with(self);
+
                 let obligations = self.nominal_obligations(did, args);
                 self.out.extend(obligations);
             }
@@ -716,7 +728,7 @@ impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> {
                 if !r.has_escaping_bound_vars() && !rty.has_escaping_bound_vars() {
                     let cause = self.cause(traits::ReferenceOutlivesReferent(t));
                     self.out.push(traits::Obligation::with_depth(
-                        self.tcx(),
+                        tcx,
                         cause,
                         self.recursion_depth,
                         self.param_env,
@@ -805,12 +817,12 @@ impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> {
                 // obligations that don't refer to Self and
                 // checking those
 
-                let defer_to_coercion = self.tcx().features().object_safe_for_dispatch;
+                let defer_to_coercion = tcx.features().object_safe_for_dispatch;
 
                 if !defer_to_coercion {
                     if let Some(principal) = data.principal_def_id() {
                         self.out.push(traits::Obligation::with_depth(
-                            self.tcx(),
+                            tcx,
                             self.cause(traits::WellFormed(None)),
                             self.recursion_depth,
                             self.param_env,
@@ -835,7 +847,7 @@ impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> {
             ty::Infer(_) => {
                 let cause = self.cause(traits::WellFormed(None));
                 self.out.push(traits::Obligation::with_depth(
-                    self.tcx(),
+                    tcx,
                     cause,
                     self.recursion_depth,
                     self.param_env,
@@ -850,6 +862,8 @@ impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> {
     }
 
     fn visit_const(&mut self, c: <TyCtxt<'tcx> as ty::Interner>::Const) -> Self::Result {
+        let tcx = self.tcx();
+
         match c.kind() {
             ty::ConstKind::Unevaluated(uv) => {
                 if !c.has_escaping_bound_vars() {
@@ -861,7 +875,7 @@ impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> {
                     ));
                     let cause = self.cause(traits::WellFormed(None));
                     self.out.push(traits::Obligation::with_depth(
-                        self.tcx(),
+                        tcx,
                         cause,
                         self.recursion_depth,
                         self.param_env,
@@ -873,7 +887,7 @@ impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> {
                 let cause = self.cause(traits::WellFormed(None));
 
                 self.out.push(traits::Obligation::with_depth(
-                    self.tcx(),
+                    tcx,
                     cause,
                     self.recursion_depth,
                     self.param_env,
@@ -895,7 +909,7 @@ impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> {
                 ));
                 let cause = self.cause(traits::WellFormed(None));
                 self.out.push(traits::Obligation::with_depth(
-                    self.tcx(),
+                    tcx,
                     cause,
                     self.recursion_depth,
                     self.param_env,
diff --git a/tests/ui/fn/fn-item-lifetime-bounds.rs b/tests/ui/fn/fn-item-lifetime-bounds.rs
deleted file mode 100644
index b80b7eade23..00000000000
--- a/tests/ui/fn/fn-item-lifetime-bounds.rs
+++ /dev/null
@@ -1,37 +0,0 @@
-//@ check-pass
-//@ known-bug: #84533
-
-// Should fail. Lifetimes are checked correctly when `foo` is called, but NOT
-// when only the lifetime parameters are instantiated.
-
-use std::marker::PhantomData;
-
-#[allow(dead_code)]
-fn foo<'b, 'a>() -> PhantomData<&'b &'a ()> {
-    PhantomData
-}
-
-#[allow(dead_code)]
-#[allow(path_statements)]
-fn caller<'b, 'a>() {
-    foo::<'b, 'a>;
-}
-
-// In contrast to above, below code correctly does NOT compile.
-// fn caller<'b, 'a>() {
-//     foo::<'b, 'a>();
-// }
-
-// error: lifetime may not live long enough
-//   --> src/main.rs:22:5
-//   |
-// 21 | fn caller<'b, 'a>() {
-//   |           --  -- lifetime `'a` defined here
-//   |           |
-//   |           lifetime `'b` defined here
-// 22 |     foo::<'b, 'a>();
-//   |     ^^^^^^^^^^^^^^^ requires that `'a` must outlive `'b`
-//   |
-//   = help: consider adding the following bound: `'a: 'b`
-
-fn main() {}
diff --git a/tests/ui/proc-macro/bad-projection.rs b/tests/ui/proc-macro/bad-projection.rs
index e633191bd31..0769a7f08c1 100644
--- a/tests/ui/proc-macro/bad-projection.rs
+++ b/tests/ui/proc-macro/bad-projection.rs
@@ -15,4 +15,5 @@ pub fn uwu() -> <() as Project>::Assoc {}
 //~^ ERROR the trait bound `(): Project` is not satisfied
 //~| ERROR the trait bound `(): Project` is not satisfied
 //~| ERROR the trait bound `(): Project` is not satisfied
+//~| ERROR the trait bound `(): Project` is not satisfied
 //~| ERROR function is expected to take 1 argument, but it takes 0 arguments
diff --git a/tests/ui/proc-macro/bad-projection.stderr b/tests/ui/proc-macro/bad-projection.stderr
index 8e0d8461849..2e8668f60de 100644
--- a/tests/ui/proc-macro/bad-projection.stderr
+++ b/tests/ui/proc-macro/bad-projection.stderr
@@ -36,6 +36,18 @@ LL | trait Project {
    | ^^^^^^^^^^^^^
 
 error[E0277]: the trait bound `(): Project` is not satisfied
+  --> $DIR/bad-projection.rs:14:1
+   |
+LL | pub fn uwu() -> <() as Project>::Assoc {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Project` is not implemented for `()`
+   |
+help: this trait has no implementations, consider adding one
+  --> $DIR/bad-projection.rs:9:1
+   |
+LL | trait Project {
+   | ^^^^^^^^^^^^^
+
+error[E0277]: the trait bound `(): Project` is not satisfied
   --> $DIR/bad-projection.rs:14:40
    |
 LL | pub fn uwu() -> <() as Project>::Assoc {}
@@ -47,7 +59,7 @@ help: this trait has no implementations, consider adding one
 LL | trait Project {
    | ^^^^^^^^^^^^^
 
-error: aborting due to 4 previous errors
+error: aborting due to 5 previous errors
 
 Some errors have detailed explanations: E0277, E0593.
 For more information about an error, try `rustc --explain E0277`.
diff --git a/tests/ui/wf/wf-fn-def-check-sig-1.rs b/tests/ui/wf/wf-fn-def-check-sig-1.rs
new file mode 100644
index 00000000000..6d9e1f38f8d
--- /dev/null
+++ b/tests/ui/wf/wf-fn-def-check-sig-1.rs
@@ -0,0 +1,44 @@
+// Regression test for #84533.
+
+use std::marker::PhantomData;
+
+fn foo<'b, 'a>() -> PhantomData<&'b &'a ()> {
+    PhantomData
+}
+
+fn extend_lifetime<'a, 'b, T: ?Sized>(x: &'a T) -> &'b T {
+    let f = foo::<'b, 'a>;
+    f.baz(x)
+    //~^ ERROR lifetime may not live long enough
+}
+
+trait Foo<'a, 'b, T: ?Sized> {
+    fn baz(self, s: &'a T) -> &'b T;
+}
+impl<'a, 'b, R, F, T: ?Sized> Foo<'a, 'b, T> for F
+where
+    F: Fn() -> R,
+    R: ProofForConversion<'a, 'b, T>,
+{
+    fn baz(self, s: &'a T) -> &'b T {
+        self().convert(s)
+    }
+}
+
+trait ProofForConversion<'a, 'b, T: ?Sized> {
+    fn convert(self, s: &'a T) -> &'b T;
+}
+impl<'a, 'b, T: ?Sized> ProofForConversion<'a, 'b, T> for PhantomData<&'b &'a ()> {
+    fn convert(self, s: &'a T) -> &'b T {
+        s
+    }
+}
+
+fn main() {
+    let d;
+    {
+        let x = "Hello World".to_string();
+        d = extend_lifetime(&x);
+    }
+    println!("{}", d);
+}
diff --git a/tests/ui/wf/wf-fn-def-check-sig-1.stderr b/tests/ui/wf/wf-fn-def-check-sig-1.stderr
new file mode 100644
index 00000000000..a93449ad3c6
--- /dev/null
+++ b/tests/ui/wf/wf-fn-def-check-sig-1.stderr
@@ -0,0 +1,15 @@
+error: lifetime may not live long enough
+  --> $DIR/wf-fn-def-check-sig-1.rs:11:5
+   |
+LL | fn extend_lifetime<'a, 'b, T: ?Sized>(x: &'a T) -> &'b T {
+   |                    --  -- lifetime `'b` defined here
+   |                    |
+   |                    lifetime `'a` defined here
+LL |     let f = foo::<'b, 'a>;
+LL |     f.baz(x)
+   |     ^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a`
+   |
+   = help: consider adding the following bound: `'a: 'b`
+
+error: aborting due to 1 previous error
+
diff --git a/tests/ui/wf/wf-fn-def-check-sig-2.rs b/tests/ui/wf/wf-fn-def-check-sig-2.rs
new file mode 100644
index 00000000000..51740dca8e9
--- /dev/null
+++ b/tests/ui/wf/wf-fn-def-check-sig-2.rs
@@ -0,0 +1,44 @@
+// Regression test for #84533 involving higher-ranked regions
+// in the return type.
+use std::marker::PhantomData;
+
+fn foo<'c, 'b, 'a>(_: &'c ()) -> (&'c (), PhantomData<&'b &'a ()>) {
+    (&(), PhantomData)
+}
+
+fn extend_lifetime<'a, 'b, T: ?Sized>(x: &'a T) -> &'b T {
+    let f = foo;
+    f.baz(x)
+    //~^ ERROR lifetime may not live long enough
+}
+
+trait Foo<'a, 'b, T: ?Sized> {
+    fn baz(self, s: &'a T) -> &'b T;
+}
+impl<'a, 'b, R, F, T: ?Sized> Foo<'a, 'b, T> for F
+where
+    F: for<'c> Fn(&'c ()) -> (&'c (), R),
+    R: ProofForConversion<'a, 'b, T>,
+{
+    fn baz(self, s: &'a T) -> &'b T {
+        self(&()).1.convert(s)
+    }
+}
+
+trait ProofForConversion<'a, 'b, T: ?Sized> {
+    fn convert(self, s: &'a T) -> &'b T;
+}
+impl<'a, 'b, T: ?Sized> ProofForConversion<'a, 'b, T> for PhantomData<&'b &'a ()> {
+    fn convert(self, s: &'a T) -> &'b T {
+        s
+    }
+}
+
+fn main() {
+    let d;
+    {
+        let x = "Hello World".to_string();
+        d = extend_lifetime(&x);
+    }
+    println!("{}", d);
+}
diff --git a/tests/ui/wf/wf-fn-def-check-sig-2.stderr b/tests/ui/wf/wf-fn-def-check-sig-2.stderr
new file mode 100644
index 00000000000..404d3cc4513
--- /dev/null
+++ b/tests/ui/wf/wf-fn-def-check-sig-2.stderr
@@ -0,0 +1,15 @@
+error: lifetime may not live long enough
+  --> $DIR/wf-fn-def-check-sig-2.rs:11:5
+   |
+LL | fn extend_lifetime<'a, 'b, T: ?Sized>(x: &'a T) -> &'b T {
+   |                    --  -- lifetime `'b` defined here
+   |                    |
+   |                    lifetime `'a` defined here
+LL |     let f = foo;
+LL |     f.baz(x)
+   |     ^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a`
+   |
+   = help: consider adding the following bound: `'a: 'b`
+
+error: aborting due to 1 previous error
+