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

expected types of closure arguments are not inferred when coercing to a fn #41755

Closed
nikomatsakis opened this issue May 4, 2017 · 4 comments · Fixed by #41838
Closed

expected types of closure arguments are not inferred when coercing to a fn #41755

nikomatsakis opened this issue May 4, 2017 · 4 comments · Fixed by #41838
Labels
A-typesystem Area: The type system E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

Type inference for the expected type of arguments doesn't work when coercing from a closure expression. For example, this code does not compile https://is.gd/FNoLWg (at least, not without an explicit x: Vec<u32> annotation):

#![feature(closure_to_fn_coercion)]

fn foo(f: fn(Vec<u32>) -> usize) { }

fn main() {
    foo(|x| x.len())
}
@nikomatsakis nikomatsakis added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-typesystem Area: The type system labels May 4, 2017
@nikomatsakis
Copy link
Contributor Author

Some brief mentoring instructions to fix this:

The expected type for closure arguments are deduced using various bits of code in src/librustc_typeck/check/closure.rs. In particular, the function deduce_expectations_from_expected_type() examines the expected type (which will be fn(Vec<u32>) -> usize in this case). Right now, it only works for trait objects like &Fn(). It needs to be extended to also check for fn pointers (the TyFnPtr variant).

Please feel free to ping me on IRC or gitter with questions! (Or ask in #rustc)

@nikomatsakis nikomatsakis added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels May 4, 2017
@ghost
Copy link

ghost commented May 4, 2017

I'd like to work on this issue

@nikomatsakis
Copy link
Contributor Author

@z1mvader great! :) Let me know if my instructions suffice, or if you feel you need some more help. I do try to keep up with GH notifications, so you can ask questions here, but you'll definitely get a "snappier" response on IRC or gitter.

@nikomatsakis
Copy link
Contributor Author

OK, so, reading a bit more into that code, I see that it's use of Binder is a bit... icky (I can say that since I wrote it). But I'm just going to go with it. Here is a slightly more detailed account of what needs to be changed:

In deduce_expectations_from_expected_type(), when we match a TyFnPtr(sig), this variable sig will be a PolyFnSig<'tcx>. This means that it's a function signature, i.e., has the types of arguments and return type.

The "Poly" bit means that this function signature may have some higher-ranked regions (e.g., fn(&i32) -> &i32, which is really shorthand for for<'a> fn(&'a i32) -> &'a i32 -- the "poly" bit is talking about the for<'a>). In fact, if you trace it out, you will see that PolyFnSig is really a type alias for Binder<FnSig>, where Binder basically corresponds to the for<'a>.

Now, this function is setup to return a (Option<ty::FnSig<'tcx>>, Option<ty::ClosureKind>). The "ickiness" I was referring at the beginning with respect to Binder is that, although the first part of this tuple is just Option<FnSig>, it really should be Option<PolyFnSig>, because this signature may still refer to bound regions. If you kind of trace it out, you'll see that we wind up adding a binder back on, but in a kind of far-apart part of the code. Sort of a mess, but let's ignore that.

So, the meaning of this return:

  • the first part of the tuple is expected fn signature. We basically want to supply the sig that we extracted from the TyFnDef, more on that in a second.
  • the second part is the "kind" of closure (by value, etc). This is irrelevant here, since it refers specifically to how the closure deals with its environment, and if we are coercing to a fn ptr, we know that there cannot be an environment. We could just return None; we could also yield Some(ty::ClosureKind::Fn), since any valid instance will satisfy the Fn trait (which is the most general trait -- a fn that can be called through a shared reference).

Therefore, I think we want to return something like (sig.skip_binder().clone(), Some(ty::ClosureKind::Fn)).

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 9, 2017
…, r=nikomatsakis

Fixed argument inference for closures when coercing into 'fn'

This fixes rust-lang#41755. The tests  `compile-fail/closure-no-fn.rs` and `compile-fail/issue-40000.rs` were modified. A new test `run-pass/closure_to_fn_coercion-expected-types.rs` was added

r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-typesystem Area: The type system E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant