RIP boxed closures #20578

japaric · 2015-01-05T15:17:46Z

This PR removes boxed closures from the language, the closure type syntax (let f: |int| -> bool = /* ... */) has been obsoleted. Move all your uses of closures to the new unboxed closure system (i.e. Fn* traits).

[breaking-change] patterns

lef f = || {}

This binding used to type check to a boxed closure. Now that boxed closures are gone, you need to annotate the "kind" of the unboxed closure, i.e. you need pick one of these: |&:| {}, |&mut:| {} or |:| {}.

In the (near) future we'll have closure "kind" inference, so the compiler will infer which Fn* trait to use based on how the closure is used. Once this inference machinery is in place, we'll be able to remove the kind annotation from most closures.

type Alias<'a> = |int|:'a -> bool

Use a trait object: type Alias<'a> = Box<FnMut(int) -> bool + 'a>. Use the Fn* trait that makes sense for your use case.

fn foo(&self, f: |uint| -> bool)

In this case you can use either a trait object or an unboxed closure:

fn foo(&self, f: F) where F: FnMut(uint) -> bool;
// or
fn foo(&self, f: Box<FnMut(uint) -> bool>);

struct Struct<'a> { f: |uint|:'a -> bool }

Again, you can use either a trait object or an unboxed closure:

struct Struct<F> where F: FnMut(uint) -> bool { f: F }
// or
struct Struct<'a> { f: Box<FnMut(uint) -> bool + 'a> }

Using |x, y| f(x, y) for closure "borrows"

This comes up in recursive functions, consider the following (contrived) example:

fn foo(x: uint, f: |uint| -> bool) -> bool {
    //foo(x / 2, f) && f(x)  // can't use this because `f` gets moved away in the `foo` call
    foo(x / 2, |x| f(x)) && f(x)  // instead "borrow" `f` in the `foo` call
}

If you attempt to do the same with unboxed closures you'll hit ""error: reached the recursion limit during monomorphization" (see #19596):

fn foo<F>(x: uint, mut f: F) -> bool where F: FnMut(uint) -> bool {
    foo(x / 2, |x| f(x)) && f(x)
    //~^ error: reached the recursion limit during monomorphization
}

Instead you should be able to write this:

fn foo<F>(x: uint, mut f: F) -> bool where F: FnMut(uint) -> bool {
    foo(x / 2, &mut f) && f(x)
    //~^ error: the trait `FnMut` is not implemented for the type `&mut F`
}

But as you see above &mut F doesn't implement the FnMut trait. &mut F should implement the FnMut and the above code should work, but due to a bug (see #18835) it doesn't (for now).

You can work around the issue by rewriting the function to take &mut F instead of F:

fn foo<F>(x: uint, f: &mut F) -> bool where F: FnMut(uint) -> bool {
    foo(x / 2, f) && (*f)(x)
}

This finally works! However writing foo(0, &mut |x| x == 0) is unergonomic. So you can use a private helper function to avoid this:

// public API function
pub fn foo<F>(x: uint, mut f: F) -> bool where F: FnMut(uint) -> bool {
    foo_(x, &mut f)
}

// private helper function
fn foo_<F>(x: uint, f: &mut F) -> bool where F: FnMut(uint) -> bool {
    foo_(x / 2, f) && (*f)(x)
}

Closes #14798

There is more cleanup to do: like renaming functions/types from unboxed_closure to just closure, removing more dead code, simplify functions which now have unused arguments, update the documentation, etc. But that can be done in another PR.

r? @nikomatsakis @aturon (You probably want to focus on the deleted/modified tests.)
cc @eddyb

nikomatsakis · 2015-01-05T15:18:22Z

my hero.

japaric · 2015-01-05T16:39:16Z

I've updated the [breaking-change] message.

alexcrichton · 2015-01-05T16:53:38Z

It may be worth noting that you can't actually get the old behavior of boxed closures today (#20575):

fn foo(f: &mut FnMut()) {
    f();
}

error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: http://doc.rust-lang.org/complement-bugreport.html
note: run with `RUST_BACKTRACE=1` for a backtrace
thread 'rustc' panicked at 'assertion failed: index < LLVMCountParams(llfn)', /home/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-linux/build/src/librustc_llvm/lib.rs:2180

This does work, but requires the feature gate, however:

#![feature(unboxed_closures)]
fn foo(mut f: Box<FnMut()>) {
    f.call_mut(());          
}

alexcrichton · 2015-01-05T16:55:58Z

src/doc/guide.md

@@ -4231,7 +4231,7 @@ arguments, really powerful things are possible.

 Let's make a closure:

-```{rust}
+```{rust,ignore}


Could we un-ignore these for landing?

cc @steveklabnik

Yes, ignoring this is bad.

@japaric would you prefer a PR from me to your PR, or do you just want to change the syntax and I can land the language after?

I'd be great if this landed with updated docs, so I'll go with the first option.

let me tackle that for you right now then ❤️

sent in japaric#1

@steveklabnik you need a stage1 compiler (built with this PR) to see the compiler errors in the doctests. Let me update the snippets and then you can update the text around. (That should be faster since I have a stage1 compiler at hand).

Oh, that's... weird. Okay.

japaric · 2015-01-05T17:04:57Z

@alexcrichton Interestingly that ICE only occurs for trait objects with zero arguments, these two work without the feature gate:

fn foo(mut f: Box<FnMut(int)>) {
    f(5)
}

fn bar(mut f: &mut FnMut(int)) {
    f(5)
}

alexcrichton · 2015-01-05T18:41:52Z

@japaric whoa nice! I had no idea! Pretend I was never here...

jroesch · 2015-01-05T19:03:22Z

👍 👍

nikomatsakis · 2015-01-05T19:50:22Z

@alexcrichton I fixed the ICE altogether and sent @japaric the commit. Hopefully it'll show up here at some point.

alexcrichton · 2015-01-05T20:50:22Z

swoon

japaric · 2015-01-05T21:08:58Z

It's ready! ICEs have been fixed (by @nikomatsakis), unignored and fixed the doctests in the guide and the reference. Last 6 commits are new.

…trait method with rust-call ABI.

@nikomatsakis

This PR removes boxed closures from the language, the closure type syntax (`let f: |int| -> bool = /* ... */`) has been obsoleted. Move all your uses of closures to the new unboxed closure system (i.e. `Fn*` traits). [breaking-change] patterns - `lef f = || {}` This binding used to type check to a boxed closure. Now that boxed closures are gone, you need to annotate the "kind" of the unboxed closure, i.e. you need pick one of these: `|&:| {}`, `|&mut:| {}` or `|:| {}`. In the (near) future we'll have closure "kind" inference, so the compiler will infer which `Fn*` trait to use based on how the closure is used. Once this inference machinery is in place, we'll be able to remove the kind annotation from most closures. - `type Alias<'a> = |int|:'a -> bool` Use a trait object: `type Alias<'a> = Box<FnMut(int) -> bool + 'a>`. Use the `Fn*` trait that makes sense for your use case. - `fn foo(&self, f: |uint| -> bool)` In this case you can use either a trait object or an unboxed closure: ``` rust fn foo(&self, f: F) where F: FnMut(uint) -> bool; // or fn foo(&self, f: Box<FnMut(uint) -> bool>); ``` - `struct Struct<'a> { f: |uint|:'a -> bool }` Again, you can use either a trait object or an unboxed closure: ``` rust struct Struct<F> where F: FnMut(uint) -> bool { f: F } // or struct Struct<'a> { f: Box<FnMut(uint) -> bool + 'a> } ``` - Using `|x, y| f(x, y)` for closure "borrows" This comes up in recursive functions, consider the following (contrived) example: ``` rust fn foo(x: uint, f: |uint| -> bool) -> bool { //foo(x / 2, f) && f(x) // can't use this because `f` gets moved away in the `foo` call foo(x / 2, |x| f(x)) && f(x) // instead "borrow" `f` in the `foo` call } ``` If you attempt to do the same with unboxed closures you'll hit ""error: reached the recursion limit during monomorphization" (see #19596): ``` rust fn foo<F>(x: uint, mut f: F) -> bool where F: FnMut(uint) -> bool { foo(x / 2, |x| f(x)) && f(x) //~^ error: reached the recursion limit during monomorphization } ``` Instead you *should* be able to write this: ``` rust fn foo<F>(x: uint, mut f: F) -> bool where F: FnMut(uint) -> bool { foo(x / 2, &mut f) && f(x) //~^ error: the trait `FnMut` is not implemented for the type `&mut F` } ``` But as you see above `&mut F` doesn't implement the `FnMut` trait. `&mut F` *should* implement the `FnMut` and the above code *should* work, but due to a bug (see #18835) it doesn't (for now). You can work around the issue by rewriting the function to take `&mut F` instead of `F`: ``` rust fn foo<F>(x: uint, f: &mut F) -> bool where F: FnMut(uint) -> bool { foo(x / 2, f) && (*f)(x) } ``` This finally works! However writing `foo(0, &mut |x| x == 0)` is unergonomic. So you can use a private helper function to avoid this: ``` rust // public API function pub fn foo<F>(x: uint, mut f: F) -> bool where F: FnMut(uint) -> bool { foo_(x, &mut f) } // private helper function fn foo_<F>(x: uint, f: &mut F) -> bool where F: FnMut(uint) -> bool { foo_(x / 2, f) && (*f)(x) } ``` Closes #14798 --- There is more cleanup to do: like renaming functions/types from `unboxed_closure` to just `closure`, removing more dead code, simplify functions which now have unused arguments, update the documentation, etc. But that can be done in another PR. r? @nikomatsakis @aturon (You probably want to focus on the deleted/modified tests.) cc @eddyb

jethrogb · 2015-01-07T22:16:26Z

Note the example in the first PR comment is wrong:

fn foo(&self, f: F) where F: FnMut(uint) -> bool;

You'll get the error use of undeclared type name F``, use instead:

fn foo<F>(&self, f: F) where F: FnMut(uint) -> bool;

See rust-lang/rust#20578

G4MR · 2015-01-14T19:58:40Z

The book for moving closure is still using boxed closures as an example: http://doc.rust-lang.org/book/closures.html

steveklabnik · 2015-01-14T20:29:51Z

They should all be unboxed. Which are boxed?

G4MR · 2015-01-15T05:02:14Z

(e.g., move || x * x).

I'm pretty sure that syntax is invalid now with 1.0?

sinistersnare · 2015-01-15T05:08:23Z

move || syntax is unboxed. proc is boxed.

eddyb · 2015-01-15T06:46:25Z

The || syntax is the intended (unboxed) closure syntax. Right now it only works when a type hint is available (including bounded generics), and the awkward specific syntax is still needed in other cases (like let id = |&:x| x;) but because of this PR, that syntax can finally be removed, after || is made to properly infer the kind of closure.

This test was ignored long ago in rust-lang#20578 when the syntax for closures was changed. The current status is that a closure with an explicit `!` return type will trigger the `unreachable_code` lint which appears to be the original intent of the test (rust-lang#16836). A closure without a return type won't trigger the lint since the `!` type isn't inferred (AFAIK). This restores the test to its original form.

rust-highfive assigned nikomatsakis Jan 5, 2015

alexcrichton reviewed Jan 5, 2015
View reviewed changes

japaric force-pushed the no-more-bc branch 2 times, most recently from 80febed to 8c195be Compare January 5, 2015 21:07

apasel422 mentioned this pull request Jan 5, 2015

unbox closures Gankra/collect-rs#65

Merged

Jorge Aparicio added 16 commits January 5, 2015 17:22

register snapshot

a291a80

std: remove remaining boxed closures

37f62ae

syntax: remove remaining boxed closures

07a8e7c

conv_did: convert to "unboxed" closure

98fda87

DecodeInlinedItem: convert to "unboxed" closures

bd9eef7

EncodeInlinedItem: convert to "unboxed" closures

0cb34a3

rustc: remove remaining boxed closures

8570f0a

typeck: remove remaining boxed closures

977e151

trans: remove remaining boxed closures

bf52e26

driver: remove unboxed closures

b4ccc90

compiletest: remove boxed closures

3744850

coretest: remove/ignore tests

18e2026

remove ty_closure

5f7f2c9

remove TyClosure

714a5b7

remove AdjustAddEnv

4e9c50e

remove mk_closure

8a6d7a6

Jorge Aparicio and others added 16 commits January 5, 2015 17:22

trans: remove dead code

6438261

typeck: remove dead code

8d0d752

fix rpass tests

ca17d08

fix cfail tests

7d5b045

fix run-make test

d6a948e

fix pretty tests

1bbeb37

fix debuginfo tests

ef72659

fix benchmarks

a9ea4d0

ignore boxed closure doctests in the guide/reference

ab0c7af

address Niko's comments

79af277

Fix ICE caused by forgotten bcx

f97b124

Correctly "detuple" arguments when creating trait object shims for a …

c98814b

…trait method with rust-call ABI.

replace f.call_mut(a, b, ..) with f(a, b, ..)

ec11f66

fix tests

a55011e

unignore and fix doctests in guide and reference

97f870a

remove more stage0 stuff

eb2506c

japaric force-pushed the no-more-bc branch from 8c195be to eb2506c Compare January 5, 2015 22:22

bors merged commit eb2506c into rust-lang:master Jan 6, 2015

drbawb mentioned this pull request Jan 6, 2015

Removes boxed closures to compile w/ latest nightly. Rust-SDL2/rust-sdl2#263

Closed

fungos mentioned this pull request Jan 7, 2015

Fix compilation with latest boxed closure changes netvl/xml-rs#58

Closed

alkor added a commit to alkor/rust-http that referenced this pull request Jan 11, 2015

Convert to unboxed closures

e7d999c

See rust-lang/rust#20578

japaric deleted the no-more-bc branch January 22, 2015 16:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RIP boxed closures #20578

RIP boxed closures #20578

japaric commented Jan 5, 2015

nikomatsakis commented Jan 5, 2015

japaric commented Jan 5, 2015

alexcrichton commented Jan 5, 2015

alexcrichton Jan 5, 2015

steveklabnik Jan 5, 2015

steveklabnik Jan 5, 2015

japaric Jan 5, 2015

steveklabnik Jan 5, 2015

steveklabnik Jan 5, 2015

japaric Jan 5, 2015

steveklabnik Jan 5, 2015

japaric commented Jan 5, 2015

alexcrichton commented Jan 5, 2015

jroesch commented Jan 5, 2015

nikomatsakis commented Jan 5, 2015

alexcrichton commented Jan 5, 2015

japaric commented Jan 5, 2015

jethrogb commented Jan 7, 2015

G4MR commented Jan 14, 2015

steveklabnik commented Jan 14, 2015

G4MR commented Jan 15, 2015

sinistersnare commented Jan 15, 2015

eddyb commented Jan 15, 2015

RIP boxed closures #20578

RIP boxed closures #20578

Conversation

japaric commented Jan 5, 2015

nikomatsakis commented Jan 5, 2015

japaric commented Jan 5, 2015

alexcrichton commented Jan 5, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

japaric commented Jan 5, 2015

alexcrichton commented Jan 5, 2015

jroesch commented Jan 5, 2015

nikomatsakis commented Jan 5, 2015

alexcrichton commented Jan 5, 2015

japaric commented Jan 5, 2015

jethrogb commented Jan 7, 2015

G4MR commented Jan 14, 2015

steveklabnik commented Jan 14, 2015

G4MR commented Jan 15, 2015

sinistersnare commented Jan 15, 2015

eddyb commented Jan 15, 2015