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

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,

tests/ui/fn/fn-item-lifetime-bounds.rs

@@ -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() {}

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

tests/ui/proc-macro/bad-projection.stderr

@@ -35,6 +35,18 @@ help: this trait has no implementations, consider adding one
 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
    |
@@ -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`.

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);
+}

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
+

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);
+}

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
+