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

a fn pointer doesn't implement Fn/FnMut/FnOnce if its return type isn't sized #100096

Merged
merged 1 commit into from
Sep 21, 2022
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
31 changes: 30 additions & 1 deletion compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,36 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
)
.map_bound(|(trait_ref, _)| trait_ref);

let nested = self.confirm_poly_trait_refs(obligation, trait_ref)?;
let mut nested = self.confirm_poly_trait_refs(obligation, trait_ref)?;

// Confirm the `type Output: Sized;` bound that is present on `FnOnce`
let cause = obligation.derived_cause(BuiltinDerivedObligation);
// The binder on the Fn obligation is "less" important than the one on
// the signature, as evidenced by how we treat it during projection.
// The safe thing to do here is to liberate it, though, which should
// have no worse effect than skipping the binder here.
let liberated_fn_ty = self.infcx.replace_bound_vars_with_placeholders(obligation.self_ty());
let output_ty = self
.infcx
.replace_bound_vars_with_placeholders(liberated_fn_ty.fn_sig(self.tcx()).output());
let output_ty = normalize_with_depth_to(
self,
obligation.param_env,
cause.clone(),
obligation.recursion_depth,
output_ty,
&mut nested,
);
let tr = ty::Binder::dummy(ty::TraitRef::new(
self.tcx().require_lang_item(LangItem::Sized, None),
self.tcx().mk_substs_trait(output_ty, &[]),
));
Comment on lines +624 to +643
Copy link
Member

Choose a reason for hiding this comment

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

Just for completeness, can you update the test to include some fn ptrs with bound vars?

nested.push(Obligation::new(
cause,
obligation.param_env,
tr.to_poly_trait_predicate().to_predicate(self.tcx()),
));

Ok(ImplSourceFnPointerData { fn_ty: self_ty, nested })
}

Expand Down
15 changes: 15 additions & 0 deletions src/test/ui/function-pointer/sized-ret-with-binder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// check-pass

#![feature(unboxed_closures)]

fn is_fn<T: for<'a> Fn<(&'a (),)>>() {}
fn is_fn2<T: for<'a, 'b> Fn<(&'a &'b (),)>>() {}

struct Outlives<'a, 'b>(std::marker::PhantomData<&'a &'b ()>);

fn main() {
is_fn::<for<'a> fn(&'a ()) -> &'a ()>();
is_fn::<for<'a> fn(&'a ()) -> &'a dyn std::fmt::Debug>();
is_fn2::<for<'a, 'b> fn(&'a &'b ()) -> Outlives<'a, 'b>>();
is_fn2::<for<'a, 'b> fn(&'a &'b ()) -> (&'a (), &'a ())>();
}
14 changes: 14 additions & 0 deletions src/test/ui/function-pointer/unsized-ret.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#![feature(fn_traits)]
#![feature(unboxed_closures)]

fn foo<F: Fn<T>, T>(f: Option<F>, t: T) {
let y = (f.unwrap()).call(t);
}

fn main() {
foo::<fn() -> str, _>(None, ());
//~^ ERROR the size for values of type `str` cannot be known at compilation time

foo::<for<'a> fn(&'a ()) -> (dyn std::fmt::Display + 'a), _>(None, (&(),));
//~^ ERROR the size for values of type `(dyn std::fmt::Display + 'a)` cannot be known at compilation time
}
35 changes: 35 additions & 0 deletions src/test/ui/function-pointer/unsized-ret.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error[E0277]: the size for values of type `str` cannot be known at compilation time
--> $DIR/unsized-ret.rs:9:27
|
LL | foo::<fn() -> str, _>(None, ());
| --------------------- ^^^^ doesn't have a size known at compile-time
| |
| required by a bound introduced by this call
|
= help: within `fn() -> str`, the trait `Sized` is not implemented for `str`
Copy link
Contributor

Choose a reason for hiding this comment

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

Random thought: I wonder if we could get the span for the str and use it for a span_help here, opportunistically (because for things like type aliases in a foreign crate, we might not be able to recover them).

= note: required because it appears within the type `fn() -> str`
note: required by a bound in `foo`
--> $DIR/unsized-ret.rs:4:11
|
LL | fn foo<F: Fn<T>, T>(f: Option<F>, t: T) {
| ^^^^^ required by this bound in `foo`
Comment on lines +11 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

The implicit Sized requirements should have their own obligation cause so that we could mention why it was required (as the code doesn't mention it, this would be mystifying the first time you encounter it).


error[E0277]: the size for values of type `(dyn std::fmt::Display + 'a)` cannot be known at compilation time
--> $DIR/unsized-ret.rs:12:66
|
LL | foo::<for<'a> fn(&'a ()) -> (dyn std::fmt::Display + 'a), _>(None, (&(),));
| ------------------------------------------------------------ ^^^^ doesn't have a size known at compile-time
| |
| required by a bound introduced by this call
|
= help: within `for<'a> fn(&'a ()) -> (dyn std::fmt::Display + 'a)`, the trait `for<'a> Sized` is not implemented for `(dyn std::fmt::Display + 'a)`
Copy link
Member Author

Choose a reason for hiding this comment

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

Interestingly this mentions for<'a> Sized, but I think that's because we print placeholder regions as if they were in binders.

= note: required because it appears within the type `for<'a> fn(&'a ()) -> (dyn std::fmt::Display + 'a)`
note: required by a bound in `foo`
--> $DIR/unsized-ret.rs:4:11
|
LL | fn foo<F: Fn<T>, T>(f: Option<F>, t: T) {
| ^^^^^ required by this bound in `foo`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
error[E0275]: overflow evaluating the requirement `fn() -> Foo {foo}: Sized`
error[E0275]: overflow evaluating the requirement `Foo: Sized`
--> $DIR/issue-53398-cyclic-types.rs:5:13
|
LL | fn foo() -> Foo {
| ^^^
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_53398_cyclic_types`)
= note: required because it appears within the type `fn() -> Foo {foo}`

error: aborting due to previous error

Expand Down