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

Closure kind inference: infer whether a closure is Fn, FnMut, etc #16640

Closed
pcwalton opened this issue Aug 20, 2014 · 10 comments · Fixed by #21899
Closed

Closure kind inference: infer whether a closure is Fn, FnMut, etc #16640

pcwalton opened this issue Aug 20, 2014 · 10 comments · Fixed by #21899
Assignees
Labels
A-closures Area: closures (`|args| { .. }`) P-medium Medium priority
Milestone

Comments

@pcwalton
Copy link
Contributor

Updated description

Closure kind inference: infer whether a closure is Fn, FnMut, etc

Original Description

Fn(int) -> int, etc.

Nominating for 1.0, P-backcompat-lang.

@pnkfelix
Copy link
Member

P-backcompat-lang, 1.0.

@pnkfelix pnkfelix added this to the 1.0 milestone Aug 28, 2014
pcwalton added a commit to pcwalton/rust that referenced this issue Sep 18, 2014
Part of issue rust-lang#16640. I am leaving this issue open to handle parsing of
higher-rank lifetimes in traits.

This change breaks code that used unboxed closures:

* Instead of `F:|&: int| -> int`, write `F:Fn(int) -> int`.

* Instead of `F:|&mut: int| -> int`, write `F:FnMut(int) -> int`.

* Instead of `F:|: int| -> int`, write `F:FnOnce(int) -> int`.

[breaking-change]
@alexcrichton alexcrichton removed this from the 1.0 milestone Nov 7, 2014
@alexcrichton alexcrichton changed the title Use the syntax in the RFC for unboxed closure sugar Implement unboxed closure inference Nov 7, 2014
@alexcrichton alexcrichton changed the title Implement unboxed closure inference Implement unboxed closure upvar inference Nov 7, 2014
@alexcrichton
Copy link
Member

I've updated the title/description/tags to all reflect the remaining pieces to do for the RFC. As pointed out by @P1start in rust-lang/rfcs#451 this isn't fully done yet in terms of a tracking issue for the RFC.

@alexcrichton alexcrichton reopened this Nov 7, 2014
@nodakai
Copy link
Contributor

nodakai commented Nov 8, 2014

Can I expect the below code to work

fn main() {
    let x = 1i;
    let f = move || {
        move || { x+1 }
    };
    println!("{}", f()());
}

once this issue is closed?

@aturon aturon mentioned this issue Nov 13, 2014
47 tasks
@pnkfelix
Copy link
Member

assigning P-high, not blocking 1.0 milestone.

@pnkfelix pnkfelix added P-medium Medium priority and removed I-nominated labels Nov 13, 2014
@aturon
Copy link
Member

aturon commented Jan 8, 2015

Nominating; we should consider the beta milestone here IMO.

@pnkfelix
Copy link
Member

pnkfelix commented Jan 8, 2015

Assigning 1.0-beta milestone to try to force this to get implemented for the beta.

@pnkfelix pnkfelix added this to the 1.0 beta milestone Jan 8, 2015
@aturon
Copy link
Member

aturon commented Jan 8, 2015

Note: this is likely required to nix the : specifier syntax on closures.

@jroesch
Copy link
Member

jroesch commented Jan 8, 2015

cc me

@nikomatsakis nikomatsakis self-assigned this Jan 26, 2015
@nikomatsakis nikomatsakis changed the title Implement unboxed closure upvar inference Closure kind inference: infer whether a closure is Fn, FnMut, etc Feb 2, 2015
@nikomatsakis
Copy link
Contributor

As of #21805 this is almost complete. The major to do item is described in that PR:

"The interaction with instantiating type parameter fallbacks leaves something to be desired. This is mostly just saying that the algorithm from rust-lang/rfcs#213 needs to be implemented, which is a separate bug. There are some semi-subtle interactions though because not knowing whether a closure is Fn vs FnMut prevents us from resolving obligations like F : FnMut(...), which can in turn prevent unification of some type parameters, which might (in turn) lead to undesired fallback. We can improve this situation however -- even if we don't know whether (or just how) F : FnMut(..) holds or not for some closure type F, we can still perform unification since we do know the argument and return types. Once kind inference is done, we can complete the F : FnMut(..) analysis -- which might yield an error if (e.g.) the F moves out of its environment."

@nikomatsakis
Copy link
Contributor

cc #21843

alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 4, 2015
This *almost* completes the job for rust-lang#16440. The idea is that even if we do not know whether some closure type `C` implements `Fn` or `FnMut` (etc), we still know its argument and return types. So if we see an obligation `C : Fn(_0)`, we can unify `_0` with those argument types while still considering the obligation ambiguous and unsatisfied. This helps to make a lot of progress with type inference even before closure kind inference is done.

As part of this PR, the explicit `:` syntax is removed from the AST and completely ignored. We still infer the closure kind based on the expected type if that is available. There are several reasons for this. First, deciding the closure kind earlier is always better, as it allows us to make more progress. Second, this retains a (admittedly obscure) way for users to manually specify the closure kind, which is useful for writing tests if nothing else. Finally, there are still some cases where inference can fail, so it may be useful to have this manual override. (The expectation is that we will eventually revisit an explicit syntax for specifying the closure kind, but it will not be `:` and may be some sort of generalization of the `||` syntax to handle other traits as well.)

This commit does not *quite* fix rust-lang#16640 because a snapshot is still needed to enable the obsolete syntax errors for explicit `&mut:` and friends.

r? @eddyb as he reviewed the prior patch in this direction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: closures (`|args| { .. }`) P-medium Medium priority
Projects
None yet
7 participants