Futures-rs: Make `futures::select!` not require it's arguments to be `Unpin`

[canndrew]

I don't know if this restriction is due to the intended semantics of futures::select! or just due to its implementation. Either way, it's a huge PITA.

The consequence of requiring select's arguments to be Unpin is that you can't use async fns directly in a call the select!, you have to pin them separately. This is usually a minor hurdle but sometimes it's a major hurdle.

I was recently in a situation where I had an async fn that takes a mutable reference:

async fn do_thing(foo: &mut Foo)

I wanted to call it using select!, but I also wanted to mutate its argument in a separate branch.

futures::select! {
    x = do_thing(&mut foo) => { ... },
    y = something_else() => {
        foo.mutate();
    },
}

This would be safe, since dropping the future returned by do_thing(&mut foo) should release the borrow. select! normally allows these kinds of cross-borrows between branches. However the Unpin requirement means this doesn't work and I had to pin the call to do_thing.

let do_thing_future = do_thing(&mut foo);
pin_mut!(do_thing_future);
futures::select! {
    x = do_thing_future => { ... },
    y = something_else() => {
        foo.mutate();
    },
}

Now the code is broken because the &mut foo borrow outlives the entire select! block. I can't see any obvious way around this. I tried re-implementing do_thing in terms of future combinators but that ended up creating a Future type that required dropping and so would also cause the borrow to outlive the select! block. In the end, the only way I could find to make this work, was to implement do_thing as a function which returns a struct which implements Future directly. Having to write poll implementations is error-prone and very cumbersome so it sucks that I couldn't find a way to avoid it.

Is the Unpin requirement strictly necessary? If it is, is there any way we could change the semantics of select! to make it unnecessary (or perhaps introduce two select! macros, one which requires Unpin and one which is somehow less featureful but doesn't require Unpin)?

[Nemo157]

select! polls the future in place, this requires it to be Unpin so that it can construct the Pin<&mut _> to it. The only way I can see to remove the Unpin requirement is to make it take the futures by value, but then it would also have to return them on all the inner branches so you could replace them, which then means moving them after they've been polled so they would have to be Unpin anyway.

I wonder if NLL allows something like:

let do_thing_future = do_thing(&mut foo);
pin_mut!(do_thing_future);
futures::select! {
    x = do_thing_future => { ... },
    y = something_else() => {
        drop(do_thing_future);
        foo.mutate();
    },
}

or, if you're not select!ing something that must remain the same in a loop you could use the select function instead.

[canndrew]

I didn't realise select! wasn't taking its arguments by value. After all, you write select! { _ = foo => _ }, not select! { _ = &mut foo => _ }. Could there be a variation of it that does? And then drops the futures as soon as one of them becomes ready so that their borrows become un-borrowed in the branches? Or would that be useless/broken for reasons I'm not seeing?

[Nemo157]

Actually, I think it's only ident futures that are polled in place, it might be possible to change the expansion so that expr futures get stack pinned internally. (That may be more confusing than helpful though).

[Nemo157]

I tried out my last idea and it appears to work: https://github.com/rust-lang-nursery/futures-rs/compare/master...Nemo157:pin-in-select.

I'm not sure how to document that it will take paths by reference and expressions by value though...

[canndrew]

Could it take paths by reference too? It wouldn't be much of a hassle to have to write:

select! {
    x = &mut foo => {...},
}

Instead of:

select! {
    x = foo => {...},
}

In fact if anything it's clearer since when I write foo I expect to be moving foo. I've always found the format! family of macros weird for not following the normal rules of Rust function application, and I like that Rust isn't like C++ in that I can always tell whether an argument is being passed by value or by reference. For format! and friends though it would never make sense to not pass by reference, so there's a case for breaking the rules and making it take references implicitly. I don't think the same case can be made for select! though.

[Nemo157]

cc @cramertj, what do you think of making select take the futures by value so it can stack pin them and require &mut foo or foo.by_ref() to poll a future in a loop?

[jesskfullwood]

I am in favour of this change. I found the current behaviour surprising given that there is no such requirement for join which semantically is similar.

[Matthias247]

I am in favour of this change. I found the current behaviour surprising given that there is no such requirement for join which semantically is similar.

There is an important difference: It's common to use futures after a select! call, while after join! you are always done with them. The most common occurence are select loops like:

while !done {
    select! {
        _ = fut => { done = true },
    }
}

[Matthias247]

An old thought I had regarding the issue is whether the select macro could act different if the passed argument is already a Future or an expression that resolves to a future. If it is Future it needs to be already pinned, since it might be polled again outside of the loop. If it's an expression then it can be evaluated and stack-pinned inside the select expression.

However I'm not 100% sure if that different behavior would be would be approachable to new users or make things more weird. For lots of users it would definitely be easier, since they then don't need to deal with pinning anymore - but once they directly try to use a future or intermediate value things might get weird.

[Matthias247]

I added support for !Unpin futures via auto-pinning expressions in #1873.

Please not that this will not fully fix this issue - only headline itself.
The given example still will fail due to lifetime/borrow-checker issues, as the following test told me:

async fn do_thing(foo: &mut i32) {}
async fn something_else() {}

#[test]
fn select_mutable_stuff() {
    block_on(async {
        let mut foo = 234;
        select! {
            x = do_thing(&mut foo).fuse() => { },
            y = something_else().fuse() => {
                foo += 5;
            },
        }
    });
}

error[E0503]: cannot use `foo` because it was mutably borrowed
  --> futures/tests/async_await_macros_1.rs:38:17
   |
35 | /         select! {
36 | |             x = do_thing(&mut foo).fuse() => { },
   | |                          -------- borrow of `foo` occurs here
37 | |             y = something_else().fuse() => {
38 | |                 foo += 5;
   | |                 ^^^^^^^^ use of borrowed `foo`
39 | |             },
40 | |         }
   | |_________- borrow might be used here, when `_0` is dropped and runs the destructor for type `futures_util::future::fuse::Fuse<impl core::future::future::Future>`