From e8472e84e3e1ef614b97896d42e0a6976fb02855 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 13 Aug 2024 17:38:24 -0400 Subject: [PATCH] Check unnormalized signature on pointer cast --- compiler/rustc_borrowck/src/type_check/mod.rs | 71 +++++++++++++++++-- ...ed-references-plus-variance-early-bound.rs | 11 +++ ...eferences-plus-variance-early-bound.stderr | 10 +++ ...d-references-plus-variance-unnormalized.rs | 17 +++++ ...ferences-plus-variance-unnormalized.stderr | 14 ++++ ...unds-on-nested-references-plus-variance.rs | 7 +- ...-on-nested-references-plus-variance.stderr | 10 +++ 7 files changed, 127 insertions(+), 13 deletions(-) create mode 100644 tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-early-bound.rs create mode 100644 tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-early-bound.stderr create mode 100644 tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-unnormalized.rs create mode 100644 tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-unnormalized.stderr create mode 100644 tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance.stderr diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 224f8d5c893..3c9a43883ad 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -1979,19 +1979,76 @@ fn check_rvalue(&mut self, body: &Body<'tcx>, rvalue: &Rvalue<'tcx>, location: L match cast_kind { CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer) => { - let fn_sig = op.ty(body, tcx).fn_sig(tcx); + let src_sig = op.ty(body, tcx).fn_sig(tcx); + + // HACK: This shouldn't be necessary... We can remove this when we actually + // get binders with where clauses, then elaborate implied bounds into that + // binder, and implement a higher-ranked subtyping algorithm that actually + // respects these implied bounds. + // + // This protects against the case where we are casting from a higher-ranked + // fn item to a non-higher-ranked fn pointer, where the cast throws away + // implied bounds that would've needed to be checked at the call site. This + // only works when we're casting to a non-higher-ranked fn ptr, since + // placeholders in the target signature could have untracked implied + // bounds, resulting in incorrect errors. + // + // We check that this signature is WF before subtyping the signature with + // the target fn sig. + if src_sig.has_bound_regions() + && let ty::FnPtr(target_fn_tys, target_hdr) = *ty.kind() + && let target_sig = target_fn_tys.with(target_hdr) + && let Some(target_sig) = target_sig.no_bound_vars() + { + let src_sig = self.infcx.instantiate_binder_with_fresh_vars( + span, + BoundRegionConversionTime::HigherRankedType, + src_sig, + ); + let src_ty = Ty::new_fn_ptr(self.tcx(), ty::Binder::dummy(src_sig)); + self.prove_predicate( + ty::ClauseKind::WellFormed(src_ty.into()), + location.to_locations(), + ConstraintCategory::Cast { unsize_to: None }, + ); + + let src_ty = self.normalize(src_ty, location); + if let Err(terr) = self.sub_types( + src_ty, + *ty, + location.to_locations(), + ConstraintCategory::Cast { unsize_to: None }, + ) { + span_mirbug!( + self, + rvalue, + "equating {:?} with {:?} yields {:?}", + target_sig, + src_sig, + terr + ); + }; + } + + let src_ty = Ty::new_fn_ptr(tcx, src_sig); + // HACK: We want to assert that the signature of the source fn is + // well-formed, because we don't enforce that via the WF of FnDef + // types normally. This should be removed when we improve the tracking + // of implied bounds of fn signatures. + self.prove_predicate( + ty::ClauseKind::WellFormed(src_ty.into()), + location.to_locations(), + ConstraintCategory::Cast { unsize_to: None }, + ); // The type that we see in the fcx is like // `foo::<'a, 'b>`, where `foo` is the path to a // function definition. When we extract the // signature, it comes from the `fn_sig` query, // and hence may contain unnormalized results. - let fn_sig = self.normalize(fn_sig, location); - - let ty_fn_ptr_from = Ty::new_fn_ptr(tcx, fn_sig); - + let src_ty = self.normalize(src_ty, location); if let Err(terr) = self.sub_types( - ty_fn_ptr_from, + src_ty, *ty, location.to_locations(), ConstraintCategory::Cast { unsize_to: None }, @@ -2000,7 +2057,7 @@ fn check_rvalue(&mut self, body: &Body<'tcx>, rvalue: &Rvalue<'tcx>, location: L self, rvalue, "equating {:?} with {:?} yields {:?}", - ty_fn_ptr_from, + src_ty, ty, terr ); diff --git a/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-early-bound.rs b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-early-bound.rs new file mode 100644 index 00000000000..16095f66da7 --- /dev/null +++ b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-early-bound.rs @@ -0,0 +1,11 @@ +static UNIT: &'static &'static () = &&(); + +fn foo<'a: 'a, 'b: 'b, T>(_: &'a &'b (), v: &'b T) -> &'a T { v } + +fn bad<'a, T>(x: &'a T) -> &'static T { + let f: fn(_, &'a T) -> &'static T = foo; + //~^ ERROR lifetime may not live long enough + f(UNIT, x) +} + +fn main() {} diff --git a/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-early-bound.stderr b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-early-bound.stderr new file mode 100644 index 00000000000..4925576e5a0 --- /dev/null +++ b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-early-bound.stderr @@ -0,0 +1,10 @@ +error: lifetime may not live long enough + --> $DIR/implied-bounds-on-nested-references-plus-variance-early-bound.rs:6:12 + | +LL | fn bad<'a, T>(x: &'a T) -> &'static T { + | -- lifetime `'a` defined here +LL | let f: fn(_, &'a T) -> &'static T = foo; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ type annotation requires that `'a` must outlive `'static` + +error: aborting due to 1 previous error + diff --git a/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-unnormalized.rs b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-unnormalized.rs new file mode 100644 index 00000000000..b842b21224d --- /dev/null +++ b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-unnormalized.rs @@ -0,0 +1,17 @@ +trait ToArg { + type Arg; +} +impl ToArg for U { + type Arg = T; +} + +fn extend_inner<'a, 'b>(x: &'a str) -> <&'b &'a () as ToArg<&'b str>>::Arg { x } +fn extend<'a, 'b>(x: &'a str) -> &'b str { + (extend_inner as fn(_) -> _)(x) + //~^ ERROR lifetime may not live long enough +} + +fn main() { + let y = extend(&String::from("Hello World")); + println!("{}", y); +} diff --git a/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-unnormalized.stderr b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-unnormalized.stderr new file mode 100644 index 00000000000..a2ac0d897b6 --- /dev/null +++ b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance-unnormalized.stderr @@ -0,0 +1,14 @@ +error: lifetime may not live long enough + --> $DIR/implied-bounds-on-nested-references-plus-variance-unnormalized.rs:10:5 + | +LL | fn extend<'a, 'b>(x: &'a str) -> &'b str { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +LL | (extend_inner as fn(_) -> _)(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/implied-bounds/implied-bounds-on-nested-references-plus-variance.rs b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance.rs index f3401f34eec..d40418a4e90 100644 --- a/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance.rs +++ b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance.rs @@ -1,15 +1,10 @@ -//@ check-pass -//@ known-bug: #25860 - -// Should fail. The combination of variance and implied bounds for nested -// references allows us to infer a longer lifetime than we can prove. - static UNIT: &'static &'static () = &&(); fn foo<'a, 'b, T>(_: &'a &'b (), v: &'b T) -> &'a T { v } fn bad<'a, T>(x: &'a T) -> &'static T { let f: fn(_, &'a T) -> &'static T = foo; + //~^ ERROR lifetime may not live long enough f(UNIT, x) } diff --git a/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance.stderr b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance.stderr new file mode 100644 index 00000000000..1ad9ba31f37 --- /dev/null +++ b/tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance.stderr @@ -0,0 +1,10 @@ +error: lifetime may not live long enough + --> $DIR/implied-bounds-on-nested-references-plus-variance.rs:6:12 + | +LL | fn bad<'a, T>(x: &'a T) -> &'static T { + | -- lifetime `'a` defined here +LL | let f: fn(_, &'a T) -> &'static T = foo; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ type annotation requires that `'a` must outlive `'static` + +error: aborting due to 1 previous error +