From 2b67f0104a9fcdb3a6aa41bceb33e62e609e6b6c Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 4 Sep 2023 17:35:51 +0200 Subject: [PATCH 1/2] check `FnDef` return type for WF --- .../rustc_trait_selection/src/traits/wf.rs | 28 +++++++++--- tests/ui/fn/fn-item-lifetime-bounds.rs | 37 ---------------- tests/ui/wf/wf-fn-def-check-sig-1.rs | 44 +++++++++++++++++++ tests/ui/wf/wf-fn-def-check-sig-1.stderr | 14 ++++++ tests/ui/wf/wf-fn-def-check-sig-2.rs | 44 +++++++++++++++++++ tests/ui/wf/wf-fn-def-check-sig-2.stderr | 15 +++++++ 6 files changed, 138 insertions(+), 44 deletions(-) delete mode 100644 tests/ui/fn/fn-item-lifetime-bounds.rs create mode 100644 tests/ui/wf/wf-fn-def-check-sig-1.rs create mode 100644 tests/ui/wf/wf-fn-def-check-sig-1.stderr create mode 100644 tests/ui/wf/wf-fn-def-check-sig-2.rs create mode 100644 tests/ui/wf/wf-fn-def-check-sig-2.stderr diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs index 7941a8fe95c8e..9b078e45c8558 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> for WfPredicates<'a, 'tcx> { fn visit_ty(&mut self, t: 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> 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> 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> 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> 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> for WfPredicates<'a, 'tcx> { } fn visit_const(&mut self, c: 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> 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> 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> 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 b80b7eade23d1..0000000000000 --- 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/wf/wf-fn-def-check-sig-1.rs b/tests/ui/wf/wf-fn-def-check-sig-1.rs new file mode 100644 index 0000000000000..621d1ac3fc1be --- /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>; + //~^ ERROR lifetime may not live long enough + f.baz(x) +} + +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 0000000000000..9501d6de8b528 --- /dev/null +++ b/tests/ui/wf/wf-fn-def-check-sig-1.stderr @@ -0,0 +1,14 @@ +error: lifetime may not live long enough + --> $DIR/wf-fn-def-check-sig-1.rs:10:13 + | +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>; + | ^^^^^^^^^^^^^ requires that `'a` must outlive `'b` + | + = help: consider adding the following bound: `'a: 'b` + +error: aborting due to 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 0000000000000..51740dca8e9be --- /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 0000000000000..70c0fd32e1bd2 --- /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 previous error + From 82789763c7c5c09c6b0481d18cf00ef67b4b6fa3 Mon Sep 17 00:00:00 2001 From: Boxy Date: Thu, 4 Apr 2024 02:14:52 +0100 Subject: [PATCH 2/2] rebase --- compiler/rustc_trait_selection/src/traits/wf.rs | 4 ++-- tests/ui/proc-macro/bad-projection.rs | 1 + tests/ui/proc-macro/bad-projection.stderr | 14 +++++++++++++- tests/ui/wf/wf-fn-def-check-sig-1.rs | 2 +- tests/ui/wf/wf-fn-def-check-sig-1.stderr | 7 ++++--- tests/ui/wf/wf-fn-def-check-sig-2.stderr | 2 +- 6 files changed, 22 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs index 9b078e45c8558..19ca147d3ad63 100644 --- a/compiler/rustc_trait_selection/src/traits/wf.rs +++ b/compiler/rustc_trait_selection/src/traits/wf.rs @@ -718,7 +718,7 @@ impl<'a, 'tcx> TypeVisitor> for WfPredicates<'a, 'tcx> { // 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); } @@ -863,7 +863,7 @@ impl<'a, 'tcx> TypeVisitor> for WfPredicates<'a, 'tcx> { fn visit_const(&mut self, c: as ty::Interner>::Const) -> Self::Result { let tcx = self.tcx(); - + match c.kind() { ty::ConstKind::Unevaluated(uv) => { if !c.has_escaping_bound_vars() { diff --git a/tests/ui/proc-macro/bad-projection.rs b/tests/ui/proc-macro/bad-projection.rs index e633191bd310f..0769a7f08c127 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 8e0d8461849b4..2e8668f60de74 100644 --- a/tests/ui/proc-macro/bad-projection.stderr +++ b/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`. diff --git a/tests/ui/wf/wf-fn-def-check-sig-1.rs b/tests/ui/wf/wf-fn-def-check-sig-1.rs index 621d1ac3fc1be..6d9e1f38f8d3e 100644 --- a/tests/ui/wf/wf-fn-def-check-sig-1.rs +++ b/tests/ui/wf/wf-fn-def-check-sig-1.rs @@ -8,8 +8,8 @@ fn foo<'b, 'a>() -> PhantomData<&'b &'a ()> { fn extend_lifetime<'a, 'b, T: ?Sized>(x: &'a T) -> &'b T { let f = foo::<'b, 'a>; - //~^ ERROR lifetime may not live long enough f.baz(x) + //~^ ERROR lifetime may not live long enough } trait Foo<'a, 'b, T: ?Sized> { diff --git a/tests/ui/wf/wf-fn-def-check-sig-1.stderr b/tests/ui/wf/wf-fn-def-check-sig-1.stderr index 9501d6de8b528..a93449ad3c63e 100644 --- a/tests/ui/wf/wf-fn-def-check-sig-1.stderr +++ b/tests/ui/wf/wf-fn-def-check-sig-1.stderr @@ -1,14 +1,15 @@ error: lifetime may not live long enough - --> $DIR/wf-fn-def-check-sig-1.rs:10:13 + --> $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>; - | ^^^^^^^^^^^^^ requires that `'a` must outlive `'b` +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 previous error +error: aborting due to 1 previous error diff --git a/tests/ui/wf/wf-fn-def-check-sig-2.stderr b/tests/ui/wf/wf-fn-def-check-sig-2.stderr index 70c0fd32e1bd2..404d3cc4513bc 100644 --- a/tests/ui/wf/wf-fn-def-check-sig-2.stderr +++ b/tests/ui/wf/wf-fn-def-check-sig-2.stderr @@ -11,5 +11,5 @@ LL | f.baz(x) | = help: consider adding the following bound: `'a: 'b` -error: aborting due to previous error +error: aborting due to 1 previous error