Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check WF of source type's signature on fn pointer cast #129021

Merged
merged 2 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 64 additions & 7 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1979,19 +1979,76 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {

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 },
Expand All @@ -2000,7 +2057,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self,
rvalue,
"equating {:?} with {:?} yields {:?}",
ty_fn_ptr_from,
src_ty,
ty,
terr
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//@ check-pass
//@ known-bug: #25860

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;
f(UNIT, x, &())
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Regression test for #129021.

static UNIT: &'static &'static () = &&();
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved

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() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: lifetime may not live long enough
--> $DIR/implied-bounds-on-nested-references-plus-variance-early-bound.rs:8: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

Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Regression test for #129021.

trait ToArg<T> {
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
type Arg;
}
impl<T, U> ToArg<T> 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);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: lifetime may not live long enough
--> $DIR/implied-bounds-on-nested-references-plus-variance-unnormalized.rs:12: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

Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
//@ check-pass
//@ known-bug: #25860

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I should probably rewrite this test to continue being a known-bug?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just make it higher-ranked in one more arg or sth...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, and also add "regression test for X"

// Should fail. The combination of variance and implied bounds for nested
// references allows us to infer a longer lifetime than we can prove.
// Regression test for #129021.

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

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: lifetime may not live long enough
--> $DIR/implied-bounds-on-nested-references-plus-variance.rs:8: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

Loading