Rust: Tracking issue for FnBox()

[abonander]

This is a tracking issue for the fn_box feature and FnBox trait, which allows a Box<FnOnce(...) -> _> closure to be invoked via the "rust-call" sugar.

I'm not sure what the primary concerns are. All I know is it'd be really, really nice if we could invoke a Box<FnOnce()> _somehow_ in safe Rust. This may or may not be the best approach.

@eddyb feel free to CC anyone who might want to comment or edit this issue to add any pertinent information.

[eefriedman]

The reason we don't want to stabilize FnBox is because we would prefer to make Box<FnOnce()> just work.

Making Box<FnOnce()> work isn't exactly controversial (there's only one obvious way to handle the construct), but it's sort of tied up with https://github.com/rust-lang/rfcs/issues/997, and more generally the issue that we want to remove the special cases for Box from the compiler.

[charlesetc]

So in the stable version of rust it's impossible to call Box<FnOnce()> and it's impossible to return a closure that's not in a box, judging from the book, because it doesn't have a known size. So how does one return a closure and call it?

[abonander]

@Charlesetc Box<Fn()> and Box<FnMut()> can both be called without a problem, because they don't require destructuring the box they're in; they only need &self or &mut self, respectively, which totally works with the current deref coercions. Which of the closure traits, Fn(), FnMut(), or FnOnce(), is implemented for a specific closure is determined by inference.

Box<FnOnce()> is a challenge because it takes self by-value, which means it needs to be moved out of the Box and onto the stack to be executed, but this doesn't work in current Rust because there's no stable way to move out of (destructure) a Box. The DerefMove trait proposed in the RFC @eefriedman linked would make this work, but the design of it is still being worked on.

The key here is that Box<FnOnce()> _does_ have a known size, but it's hidden behind the Box pointer so that anything implementing FnOnce, _regardless of its size_, can fit in the same spot. The v-table in the trait object directs the processor to the concrete implementation of the trait (in this case, the closure body) for that specific object. All the details about the object, its size, fields, etc., are coded into that implementation.

[charlesetc]

Oh that makes more sense, thanks!

[kosta]

I don't want to be pushy, but I think this is really a major missing piece in Rusts "Closures" story - and not really mentioned on in the rust book.

I understand that you want to remove special-cases for Box in the compiler and I agree that that's a worthy goal. But when will that land?

If we can have Box<FnOnce> right now for the price of "a bit more Box magic" until the no-special-case-for-Box story is complete, then is that maybe the right price to pay?
(I couldn't say, I would just profit from it as a Rust user)

[alexcrichton]

Nominating for discussion in 1.6

[nikomatsakis]

cc me

[alexcrichton]

:bell: This issue is now entering its cycle-long final comment period :bell:

The libs team discussed this and we have two possible routes for this right now:

1. The in-tree copy could be stabilized with the intention of immediately deprecating once by-value DST works. The in-tree copy has the benefit of "being variadic" and having the paren sugar in bounds.
2. The in-tree copy could be deprecated and a crates.io crate could provide a suite of traits like FnBox1, FnBox2, etc. This has the benefit of not polluting the standard library but is less ergonomic to use.

We're a little undecided on which route to take, so some comments would be welcome!

[abonander]

I'd vote for the former. We have enough micro-crates in the ecosystem and this is a common enough pattern that it's worth supporting in the stdlib.

Edit: I want to add that I do have a use for this. I need it for one-off callbacks in wingui. Right now I'm having to wrap FnOnce callbacks in an Option and move it into an FnMut closure so I can call it from a box. It's not very ergonomic.

I don't want to have to add _another_ crate to my build because that seems like a very heavyweight solution just to get a trait or two.

[sfackler]

@cybergeek94 you can do a bit better than that with some boilerplate: https://github.com/sfackler/r2d2/blob/master/src/thunk.rs but it's a bit of a pain.

[abonander]

It's clunky any way you spin it.

[bstrie]

Given that we all understand that FnBox is a will-inevitably-be-deprecated-hopefully-sooner-rather-than-later hack (albeit a useful one), I would like commitment from the Rust devs on _some_ kind of timeframe for fixing the underlying issues before letting this be stabilized. Being able to say "this will be deprecated within the next year" is much more reassuring than "this will be deprecated, uhm, iunno, sometime".

[sfackler]

What's wrong with "this will be deprecated, uhm, iunno, sometime"? If we don't have a timeline for by-value trait objects, why would we _not_ want to have this bridge?

[dlight]

I don't want to have to add another crate to my build because that seems like a very heavyweight solution just to get a trait or two.

Well, the "it lives on crates.io until it's stabilized" pattern has been applied to other cases, it's a stopgap solution that's cleaner than introducing something with the intent of deprecating it.

[sinclairzx81]

Hi,

With the current implementation of FnBox, I note that drop() is not implicitly called for captured members at the end of a boxed closure, which may (or may not) be a unintended source of leaking memory if relying solely on Rust's move semantics. I have produced a gist to demonstrate what i believe to be going on.

https://play.rust-lang.org/?gist=aa96e9e0fe78a09af2eb&version=nightly
https://gist.github.com/aa96e9e0fe78a09af2eb

I am not sure of the future of FnBox, (certainly would love to see a full blown Box<FnOnce> implementation if under review for a 1.6 release), but in the interim, I wonder if its possible to address this specifically to bring things inline.

Thanks all

[eefriedman]

@sinclairzx81 Filed #29946.

[sinclairzx81]

@eefriedman Brilliant, thank you.

[nikomatsakis]

I think I would stabilize this, personally. My reasoning is:

1. Anything we can do to make things more ergonomic here is good, until such time as Box<FnOnce()> works properly.
2. The original idea of FnBox was that it would serve as a place-holder for where you would use Box<FnOnce> such that, in the glorious future, you can basically s/FnBox/FnOnce/ and Everything Just Works. I think this is mostly true now. This will be rather untrue if we move things to a crates.io crate. Obviously I don't really expect anyone to run sed, but I think having FnBox work as much like FnOnce as we can makes it easier to explain to newcomers: "ah, Box<FnOnce> doesn't quite work yet, but you can do Box<FnBox> for now, and it mostly works the same except that it requires a box".

Basically, our story here is not great, but I don't see any reason to make it worse, just to avoid a deprecated item in the standard library.

[alexcrichton]

The libs team discussed this during triage yesterday and the decision was to stabilize. @bluss brought up a good point, however, that Box<FnMut<A>> doesn't implement FnMut<A> for coherence reasons related to FnBox which may be good to sort out, however. Concretely, an impl like

impl<F: ?Sized, A> FnMut<A> for Box<F> where F: FnMut<A> {
    ...
}

does not work. An impl of FnMut requires an impl of Fn which in turn requires an impl of FnOnce:

impl<F: ?Sized, A> FnOnce<A> for Box<F> where F: FnOnce<A> {
    ...
}

Unfortunately this impl can't be written for two reasons:

1. Unsized types by value don't work, so we can't use .call_once or * to move out of the box if F: ?Sized.
2. This impl is a coherence violation with FnOnce<A> for Box<FnBox<A>>.

[eddyb]

@nikomatsakis What's blocking Box<FnOnce()> from "just working"?
FnBox seems like a lot of work for a band-aid.

[alexcrichton]

Ok, so I'm wary to stabilize FnBox if it prevents us from later having the impls for Box<FnMut()> to also implement FnMut(), and to drill down a bit I've come up with this small sample:

#![crate_type = "lib"]
#![feature(fundamental)]

#[fundamental]
trait FnBox<A> {}
#[fundamental]
trait FnOnce<A> {}

struct Box<T: ?Sized>(T);

impl<A> FnOnce<A> for Box<FnBox<A>> {}

impl<F: FnOnce<A> + ?Sized, A> FnOnce<A> for Box<F> {}

This represents basically the traits that are in play here, but doesn't worry about the unsized types by value problem. Currently this code yields a coherence violation:

<anon>:11:1: 11:39 error: conflicting implementations for trait `FnOnce` [E0119]
<anon>:11 impl<A> FnOnce<A> for Box<FnBox<A>> {}
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<anon>:11:1: 11:39 help: see the detailed explanation for E0119
<anon>:13:1: 13:55 note: note conflicting implementation here
<anon>:13 impl<F: FnOnce<A> + ?Sized, A> FnOnce<A> for Box<F> {}
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to previous error

Yet I would think that because of the fundamental attribute the compiler _should_ be able to know that the type FnBox<A> does not implement FnOnce<A> so the two impls can never overlap. @nikomatsakis mentioned that #19032 may come into play here.

For now I'm not going to stabilize FnBox while we sort this out, but it's certainly a candidate for backporting quickly!

[sfackler]

@eddyb DST by value

[glaebhoerl]

"DST by value" or its explicit / first-class formulation, &move.

[Ericson2314]

@glaebhoerl they are different:

• fn foo(&move DST) -> SST doesn't copy bits into its stack frame
• fn foo(DST) -> SST does copy bits.
• fn foo(SST) -> &move DST better be refining a move reference as it's stack frame doesn't live long enough.
• fn foo(SST) -> DST copies bits into receiver.

Now for making a realistic API &move and &fillMeIn may be used under the hood, just as they are for certain return values today, but that is an implementation detail. Out pointers are not very ergonomic.

[alexcrichton]

If it's ok with others, let's keep this issue focused on FnBox, not on topics like DST-by-value, why FnOnce() for Box<F: FnOnce()> doesn't work today, &move, etc.

The specific issue at hand that we are worried about is that stabilizing FnBox as-is may provide a coherence violation to later have FnOnce() for Box<F: FnOnce()> in the standard library (which is something we'd like to do). It doesn't work today, but we can somewhat safely assume that it will _eventually_ work.

[Ericson2314]

@sfackler Is it that though? We are not trying to do box make_fn_once_dst(), but rather my_boxed_closure as Box<FnOnce()>. I thought the problem was that traits taking self by-value were not "object safe". As long the concrete type implementing FnOnce is a SST, I don't see a problem. I forget if FnOnce does or doesn't have a sized bound, but if it doesn't, Box<FnOnce() + Sized> should work fine with a little effort.

[alexcrichton]

@Ericson2314 sorry, to be clear I meant that let's have discussions like that on other threads/issues as it's not relevant to FnBox here.

[Ericson2314]

@alexcrichton Hmm? My previous comment on &move vs by-value DST was off topic I'll readily admit, but isn't trying to clarify what the underlying problem with Box<FnOnce> quite relevant to deciding whether FnBox is worth stabilizing?

[aturon]

@Ericson2314 I think the subtext here is that any way of making Box<FnOnce> work is going to require some non-trivial new language work, and thus will take quite some time to design, implement, and stabilize. Meanwhile, as @nikomatsakis points out, FnBox makes things a bit more ergonomic in the meantime, and is easily deprecated later.

In short, the team has already decided to move forward with stabilization in principle, with eventual deprecation planned. All that's left to discuss here is whether that stabilization/deprecation would somehow cause us problems later on.

[Ericson2314]

Sorry, I misread my notification and thought eddyb's comment was the first I hadn't read and thus stabilization had not been decided upon just yet.

Technically, I see also that per our ABI, the caller needs to be aware of static size of the argument, so Box<FnOnce() + Sized> _isn't_ easier than Box<FnOnce() + ?Sized>.

[alexcrichton]

:bell: This issue is now remaining in final comment period for stabilization :bell:

We'd like to specifically sort out this question of whether we can add these impls in a stable fashion today and _still_ be able to have the full suite of other Fn trait implementations in the future. The coherence error in my comment above is something that we're specifically concerned about.

@nikomatsakis, do you have any thoughts here about that setup? Is that expected to give a coherence violation?

[abonander]

We have two major problems with FnBox() and HRTB, dunno how it impacts stabilization but I personally would like to see them fixed first:

• #28222
• #25647

If manually invoking via FnBox::call_box() continues to remain unstable as with Fn::call() and FnMut::call_mut(), then this makes FnBox() unusable with HRTB on the stable channel even after being stabilized.

[nikomatsakis]

@alexcrichton so I investigated the coherence error you were describing in this comment. As I suspected initially, the problem is due to #19032. In particular, coherence is required to check that, for all types A:

FnBox<A>: !FnOnce<A>

(Here I am using the notation from RFC rust-lang/rfcs#1148.) Normally, it would be able to conclude this, because:

• there is no impl of FnOnce for FnBox; and,
• FnBox is a crate-local type OR FnOnce is fundamental (hence negative reasoning is allowed)

However, it decides that even despite that, it can't say for certain, because it fears that some other crate will come along with an impl like this:

// downstream crate:
struct MyStruct;
impl FnOnce<MyStruct> for FnBox<MyStruct> { }

Now, in fact, attempting to define such an impl fails the orphan check http://is.gd/e8tZc9. (Interestingly, I think it fails the orphan check precisely _because_ FnBox is not currently declared fundamental, which means that the author of FnBox reserves the right to add more impls, basically, including an impl for all T.)

I've been thinking for some time that we could fix #19032 if we made better advantage of the orphan rules. In particular, the current overlap checking code predates fundamental and does not understand the limitations that the orphan check is imposing (as we can see above). However, I think what I would like to do is to make those improvements in tandem with an improved version of negative reasoning like rust-lang/rfcs#1148. (Note to self: write some more comments on that RFC with latest thinking.)

I'm not yet sure what my conclusion is from this investigation. On the one hand, I'm 95% sure that there doesn't have to be a coherence issue here. On the other hand, I'm 5% uncertain, and I'd feel better if we resolved that 5% uncertainty before we stabilized FnBox.

[alexcrichton]

Ok, cool, thanks for the investigation @cybergeek94 and @nikomatsakis!

[alexcrichton]

The libs team discussed this in triage recently and the decision was to punt this for now due to the concerns brought up.

[alexcrichton]

cc https://github.com/rust-lang/rust/issues/33811, a possible bug for this

[lspitzner]

In case somebody else runs into this, it might be worth mentioning the "boxfnonce" crate that provides a safe+stable wrapper for this use-case.
I don't claim to understand the details of this issue and don't know how good of solution that crate provides.

[steveklabnik]

Just as a note, this issue is now going to end up being in the example of the final chapter of the book's second edition. We can just explain it and use the workarounds; it is the final chapter after all. But it is a small bit of tarnish right at the end, which is slightly unfortunate.

[bstrie]

@steveklabnik Use it as an example of how Rust is still a work-in-progress, and provide an on-ramp for people who have finished the book to come work on Rust itself. :)

[bstrie]

With the recent impl Trait stabilization, is there anything new to discuss? I figure that impl Trait alone, especially in its current conservative form, is far from obviating the need for FnBox (though at least now we can return FnOnce from functions and call them), but given how many years it's been since this issue was opened I figure it's worth getting a survey of what the current thinking is. For example, would extending impl Trait to type position solve all of this (i.e. struct Foo { x: impl FnOnce() }, let v: Vec<impl FnOnce() = ... (hm, I suppose there's some problems with that last one :P )), or are we still looking towards additional mechanisms like &move or "by-value DST" (which I don't remember ever even seeing an RFC for...)?

[bstrie]

One thing to consider: given that we already overload the call operator to different methods depending on type (call, call_mut, call_once), would it be feasible to take the call_box method from FnBox, stick it into the FnOnce trait, and have the compiler itself distinguish between call_once and call_box like it already does for the other closure calling methods? It would make it so that every Box<FnOnce> would be immediately callable today without anyone using a different/unstable type, and since call_box would remain hidden behind the call operator we wouldn't need to stabilize anything (and it's not like the Fn methods are going to be stable anytime in the foreseeable future anyway). It may not make it possible to move out of a Box<FnOnce>, but it's not like it's possible to move out of a Box<Fn> either, and it would let us deprecate FnBox. Thoughts?

[kennytm]

Given that we now support arbitrary self type, I wonder if we will need things like FnRc or FnPinMut.

[glaebhoerl]

@bstrie "By-value DST" was not only RFCed, but accepted, under the name "unsized rvalues". :)

(No implementation yet though, as far as I'm aware.)

[bstrie]

@kennytm I don't think there's any use case for Rc<FnOnce>, because you wouldn't be able to do anything with it unless the refcount is 1, which is sort of against the point of Rc. AFAICT it would be equivalent in behavior to having some code in a destructor on Foo and then doing Rc<Foo>. And I don't think that PinMut<FnOnce> makes sense either, because you'd never be able to call it since AFAIK it's basically &mut FnOnce, which also can't be called.

Generalizing this to other potential smart pointers, the underlying problem here is that there's no way at all to store a FnOnce that allows it to be called. If Box<FnOnce> ever works, it means that, in the worst-case scenario, they would just do Blah<Box<FnOnce>> rather than just Blah<FnOnce>. That seems like it would be an acceptable temporary situation until a proper solution was found, rather than coming up with a menagerie of new Fn traits.

[vorner]

@bstrie exactly, the Vec<impl Trait> (or mpsc with it) will not work ‒ for example, if you want to send unknown tasks from one thread to another, they're going to be different. impl Trait allows not to name the type, but they would still have to be the same type.

[bstrie]

Precisely, but (and this is probably getting off-topic) I was curious whether or not we would want to make all anonymous impl Foo types be considered interchangeable iff, for each specific trait Foo, they have the same size; this would (among other things) let Vec<impl Trait> work both technically and conceptually, and would let us do let v: Vec<impl FnBox> as long as each closure inserted into v closed over elements whose sizes come to the same sum (e.g. a vector could contain multiple closures if they all close over nothing). Of course this still wouldn't let anyone use those closures until we had unsized rvalues... but at the same time it would make unsized rvalues strictly more useful as well. Anyway, I'm still curious whether my above proposal to make call_box special (temporarily) would suffice for most of this.

[cuviper]

@bstrie Wouldn't that make it a breaking change for any type to change its size? That's already somewhat true when mem::transmute is in use, but what you propose seems more fundamental.

[bstrie]

Hm, probably. :) I can imagine mitigation strategies, but this is an involved topic that deserves its own internals thread or RFC, rather than clogging up this one. We only got on this tack because I brought up impl Trait as part of asking the question "what's next?", to which the answer appears to be the unsized rvalues RFC.

[mark-i-m]

Precisely, but (and this is probably getting off-topic) I was curious whether or not we would want to make all anonymous impl Foo types be considered interchangeable iff, for each specific trait Foo, they have the same size; ...

I think I would be against this. It sounds like a weird form of subtyping where forall (T: Trait) { T <: impl Trait }. I think when people want this sort of relationship, they should explicitly use a Trait object.

[vorner]

That's actually an interesting idea. Something like a Box<dyn Trait>, but without the heap allocation, one with configurable maximal size. One could put a trait object into it (so long as it would fit) and use it. I wonder if it could be implemented as a crate, with some amount of unsafe code.

But that's completely ortogonal to the problem here I guess. This one still wouldn't allow to call the thing and it would still benefit from some kind of DerefMove or something, just like what would help Box<FnOnce> to be called.

[mark-i-m]

Something like a Box, but without the heap allocation

&dyn Trait

[vorner]

&dyn Trait

No. Something owning, holding the data inline, something like ArrayVec ‒ you specify the max capacity and it acts like a vector, but stores everything inline on the stack. So this one, you'd say there's capacity of so many bytes and you could move an object implementing the given trait inside it and use it as a trait object.

Anyway, that's probably off-topic here.

[earthengine]

My considerations:

• There is no harm to stablize FnBox and make it works the same way the other FnXXX traits works.
• It is consistant with the different "fundamental" types: T, &mut T, &T and box T.
• FnBox should imply FnOnce, and FnMut should imply FnBox. The implementation is trivial. (would this be a breaking change?)

and also

• We can get Box<dyn FnOnce> just work by allowing &mut Option<dyn FnOnce> a trait object, and &mut Option<Self> a valid method receiver. See this
• By-value DST can be implemented later and should not be affected by this

Conclusion: We can have the best of both worlds: have FnBox stable and also allow Box<FnOnce>, and nothing should be affected by by-value DST RFC.

[eddyb]

sigh Unsized arguments are such a small part of by-value DSTs - they aren't even by-value DSTs, since you can always pass those arguments by reference!
We had Box<FnOnce()> working ages ago but then @nikomatsakis removed support for it.

EDIT: there are some complications with borrowck, but they would presumably be solved by borrowing the unsized argument "uniquely" for the duration of the call, instead of moving it.

[aidanhs]

You can use https://crates.io/crates/boxfnonce but it appears not to work for arguments taken by reference.

This issue is a bit frustrating, as it has a fair negative impact on the ease of structuring APIs so they accurately reflect intended ownership constraints.

[Reconcyl]

@aidanhs As far as I can tell, neither does FnBox.

Edit: Never mind; I didn't realize it was an open issue and that you can use the .call_box() desugaring instead.

[aidanhs]

Just hit this problem (again!) from another angle:

fn main() {
    let x = |_: usize| {};
    let y: Box<Fn(usize)> = Box::new(|_: usize| {});

    let o = Some(0);
    o.map_or((), x);
    o.map_or((), y);
}

https://play.rust-lang.org/?gist=dbf17ca1c5b1bcce83156127d5c5b8af&version=stable&mode=debug&edition=2015

The solution is to turn o.map_or((), y); into o.map_or((), &*y);, but the error message is pretty incomprehensible to anyone not familiar with this issue (see also #46200).

[earthengine]

53469 was caused by recent relax on calling Box<dyn FnOnce>.

Playground

fn foo(f: Box<dyn FnOnce()>) {
    f()
}
fn main() {}

causes Compiler Unexpected Panic.

[memoryruins]

fn foo(f: Box<dyn FnOnce()>) {
    f()
}
fn main() {}

is no long a panic on stable/nightly 2015/2018

error[E0161]: cannot move a value of type (dyn std::ops::FnOnce() + 'static): the size of (dyn std::ops::FnOnce() + 'static) cannot be statically determined
 --> src/main.rs:2:5
  |
2 |     f()
  |     ^

error: aborting due to previous error

53575

[SimonSapin]

As #54183 is merged, <dyn FnOnce>::call_once is callable now!

Calling Box<dyn FnOnce()> seems to work on Nightly:

#![feature(unsized_locals)]
fn a(f: Box<dyn FnOnce()>) { f() }

(Playground)

I think this removes the need for FnBox. Am I missing something, or should we deprecate and eventually remove?

@rfcbot fcp close

[rfcbot]

Team member @SimonSapin has proposed to close this. The next step is review by the rest of the tagged teams:

• [x] @Kimundi
• [x] @SimonSapin
• [x] @alexcrichton
• [x] @dtolnay
• [x] @sfackler
• [ ] @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[earthengine]

@simartin Is it too early to close? although your example seems to work, but when you actually try to call it,

#![feature(unsized_locals)]
fn a(f: Box<dyn FnOnce()>) { f() }

fn main() {
    let f = ||{ println!("Hello!") };
    a(Box::new(f))
}

Playground

you get ICE

   Compiling playground v0.0.1 (/playground)
thread 'main' panicked at 'not yet implemented: by-value trait object is not yet implemented in #![feature(unsized_locals)]', librustc_codegen_llvm/abi.rs:309:21
note: Run with `RUST_BACKTRACE=1` for a backtrace.

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.31.0-nightly (4bd4e4130 2018-10-25) running on x86_64-unknown-linux-gnu

note: compiler flags: -C codegen-units=1 -C debuginfo=2 --crate-type bin

note: some of the compiler flags provided by cargo are hidden

error: Could not compile `playground`.

To learn more, run the command again with --verbose.

Maybe it is just because the playground used an outdated nighly (10-25 in message, 10-29 already now)?

[qnighy]

Maybe it is just because the playground used an outdated nighly (10-25 in message, 10-29 already now)?

Yes, the newest nightly compiles your example.

The ICE message (that is resolved and removed in #54183) was emitted in codegen phase, so one would not have seen this unless the main function actually used it. That's the difference between your example and examples that have already compiled before #54183.

[earthengine]

Fine. Hopefully the "Nightly" at playground wouldn't become "Weekly" or worse as it is still 10-25 right now.

[rfcbot]

:bell: This is now entering its final comment period, as per the review above. :bell:

[earthengine]

The same code I posted above compiles (and runs), but when you run the miri check it gives ICE in the playground.

   Compiling playground v0.0.1 (/playground)
thread 'main' panicked at 'assertion failed: self.meta.is_none()', librustc_mir/interpret/place.rs:142:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.32.0-nightly (13dab66a6 2018-11-05) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z always-encode-mir -Z mir-emit-retag -Z mir-opt-level=0 -C codegen-units=1 -C debuginfo=2 --crate-type bin

note: some of the compiler flags provided by cargo are hidden

error: Could not compile `playground`.

To learn more, run the command again with --verbose.

Where should I report issue for the miri check?

[alexreg]

@earthengine Report it as a separate issue (referencing this one) on this repo.

[mark-i-m]

Perhaps somebody should add a test for that case before we FCP?

[SimonSapin]

I view FCP as an opportunity for anyone to object to a proposal, or to some details of it. At this point I believe there is strong consensus that we can and will make Box<dyn FnOnce()> work eventually, it’s "only" a matter of implementation effort.

So, while it’d probably be premature to deprecate FnBox right now, I think FCP to decide we want to do it once the replacement works is fine.

[eddyb]

that we can and will make Box<dyn FnOnce()> work eventually, it’s "only" a matter of implementation effort.

This seems confusing to me. It's already fully implemented, isn't it?
(ignoring the miri bug that @earthengine found - which is not relevant to Rust itself)

[SimonSapin]

I meant "implementation" as opposed to design decisions, so fixing bugs is part of it.

[eddyb]

Ah, sure, I'm only trying to point out that the only bug we're aware of relates to miri supporting this, which shouldn't block anything, it's not relevant to the language itself.

[rfcbot]

The final comment period, with a disposition to close, as per the review above, is now complete.

[gnzlbg]

This issue (fixing Box<dyn FnOnce>) is the only hard blocker preventing the libtest crate from working on stable Rust.

Playground:

fn foo(x: Box<dyn FnOnce()>) { x(); }

errors with:

error[E0161]: cannot move a value of type dyn std::ops::FnOnce(): the size of dyn std::ops::FnOnce() cannot be statically determined
 --> src/lib.rs:2:5
  |
2 |     x();
  |     ^

error: aborting due to previous error

[sfackler]

That is not a hard blocker. You can easily build a one-off trait to cover the boxed once closure use case.

[gnzlbg]

Do you have an example or is that documented somewhere?

This is the closest thing I know (playground):

trait MyFnBox {
    fn call_box(self: Box<Self>) -> ();
}

impl<T> MyFnBox for T where T: FnOnce() -> () {
    fn call_box(self: Box<Self>) -> () {
        (*self)()
    }
}

fn foo(b: Box<dyn FnOnce()>) {
    b.call_box()
}

But it doesn't work (dyn FnOnce(): ?Sized, and I don't know how to call that in a trait impl since one can't move that out of the box).

[sfackler]

Does this API specifically require taking exactly the type Box<dyn FnOnce()>?

[gnzlbg]

@sfackler I haven't spent too much time trying to use Box<dyn FnMut()> but that was the first thing I attempted and it does requires significant refactorings - it requires changing many APIs that rely on being able to move the environment into the closure to take references to the environment, which must be kept alive somehow else. I didn't see them through because I thought these refactorings were too invasive, but I'll take a PR that does them if it gets the library running on stable.

[gnzlbg]

@sfackler so I performed a lot of refactorings since I tried that, and I just tried it again, and it all worked with minimal changes - sorry for the noise =/

[earthengine]

@gnzlbg I think @sfackler means you can change the API to use Box<dyn MyFnBox> before Box<dyn FnOnce> can be used in stable.

[crlf0710]

Really wish #55431 can be revived and pushed forward.

[eddyb]

@gnzlbg Where is this requirement for libtest tracked, or where in the source is it needed? I'm not sure but I would suspect you might want your own trait anyway, that is implemented for closures but also allows manual implementations.

Alternatively, you could potentially have a wrapper that exposes only a constructor with an F: FnOnce(...) -> ... bound, and internally uses a private trait that works like FnBox.

[crlf0710]

impl FnOnce for Box<FnOnce()> will be stable in 1.35.

[SimonSapin]

PR to add a deprecation warning: https://github.com/rust-lang/rust/pull/61113.

We can remove FnBox entirely and close this issue a couple release cycles after this lands.