Add (unstable) FnBox trait as a nicer replacement for `Thunk`. #23939

nikomatsakis · 2015-04-01T14:14:14Z

Add (unstable) FnBox trait as a nicer replacement for Thunk. The doc comment includes a test that also shows how it can be used.

f? @aturon
f? @alexcrichton

rust-highfive · 2015-04-01T14:14:24Z

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

nikomatsakis · 2015-04-01T14:20:00Z

r? @alexcrichton

bors · 2015-04-01T14:49:15Z

☔ The latest upstream changes (presumably #23936) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis · 2015-04-01T15:50:53Z

Hmm, encountered an obstacle. Investigating now.

nikomatsakis · 2015-04-01T16:15:20Z

OK, so, there is an obstacle that I'm not sure how best to rectify, though it's also not a strict blocker. Just make the FnBox story less good than I hoped it was. (Of course, FnBox has always been seen as a kind of temporary work-around.)

If you have a type like x: Box<FnBox(&mut Foo)> (note the bound region there), you can't call it with x() notation, you must write x.call_box((arg,)). The reason is that the impl is for Box<FnBox<A>> which is less polymorphic than Box<for<'a> FnBox<(&'a mut Foo,)>>. The compiler's normal "instantiate polymorphic region" logic doesn't help here, because the call is being redirected through an impl, which requires that Box<FnBox<A>> == Box<for<'a> FnBox<(&'a mut Foo,)>> for some A that is just not possible.

There are two workarounds. You can manually upcast:

let x: Box<FnBox<(&mut Foo,), Output=()>> = x;

which works because Box is covariant, or just use call_box directly.

(Interestingly, this would be a kind of use case for variance on traits (that I just opened a PR to remove) except that FnBox winds up being invariant due to associated types, kind of proving my point that it's not buying us as much as it should.)

alexcrichton · 2015-04-01T16:39:11Z

src/libtest/lib.rs

I think this may need to be "FIXME" to get past make tidy

alexcrichton · 2015-04-01T16:41:01Z

Nice! r=me with a few nits

I'm ok with having to use unstable syntax to use an unstable feature :)

aturon · 2015-04-01T18:26:25Z

LGTM as well. We can discuss the stabilization story at some point during the beta period.

comment includes a test that also shows how it can be used.

for `Box<FnBox()>`. I found the alias was still handy because it is shorter than the fully written type. This is a [breaking-change]: convert code using `Invoke` to use `FnBox`, which is usually pretty straight-forward. Code using thunk mostly works if you change `Thunk::new => Box::new` and `foo.invoke(arg)` to `foo(arg)`.

nikomatsakis · 2015-04-01T19:46:25Z

@bors r=alexcrichton cade32a

nikomatsakis · 2015-04-01T20:03:51Z

@bors r=alexcrichton 8eed73f

Conflicts: src/liballoc/boxed.rs

rust-highfive assigned brson Apr 1, 2015

nikomatsakis changed the title ~~[RFC] Add (unstable) FnBox trait as a nicer replacement for Thunk.~~ [WIP] Add (unstable) FnBox trait as a nicer replacement for Thunk. Apr 1, 2015

nikomatsakis assigned alexcrichton and unassigned brson Apr 1, 2015

nikomatsakis force-pushed the fn-box branch from 293643a to 57cbb70 Compare April 1, 2015 14:20

nikomatsakis force-pushed the fn-box branch from 582d8be to 971774d Compare April 1, 2015 16:35

alexcrichton reviewed Apr 1, 2015
View reviewed changes

src/libtest/lib.rs Outdated

Copy link

Member

alexcrichton Apr 1, 2015

Choose a reason for hiding this comment

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

I think this may need to be "FIXME" to get past make tidy

nikomatsakis force-pushed the fn-box branch 2 times, most recently from 1d599ca to aa4d829 Compare April 1, 2015 18:02

aturon mentioned this pull request Apr 1, 2015

Stabilization for 1.0-beta #22500

Closed

91 tasks

nikomatsakis added 2 commits April 1, 2015 14:40

Add (unstable) FnBox trait as a nicer replacement for Thunk. The doc

ed63d32

comment includes a test that also shows how it can be used.

nikomatsakis force-pushed the fn-box branch from aa4d829 to cade32a Compare April 1, 2015 19:45

nikomatsakis changed the title ~~[WIP] Add (unstable) FnBox trait as a nicer replacement for Thunk.~~ Add (unstable) FnBox trait as a nicer replacement for Thunk. Apr 1, 2015

Remove TODO

8eed73f

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Apr 1, 2015

rollup merge of rust-lang#23939: nikomatsakis/fn-box

9bb05fd

Conflicts: src/liballoc/boxed.rs

alexcrichton merged commit 8eed73f into rust-lang:master Apr 2, 2015

nikomatsakis deleted the fn-box branch March 30, 2016 16:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add (unstable) FnBox trait as a nicer replacement for `Thunk`. #23939