Why `Unpin` is required for `time::FutureExt`?

rustasync / runtime

Empowering everyone to build asynchronous software

https://docs.rs/runtime

Apache License 2.0

862 stars 28 forks source link

Why `Unpin` is required for `time::FutureExt`? #65

Closed oxalica closed 5 years ago

oxalica commented 5 years ago

It seems not necessary since the Future is just a normal field of Timeout, according to the docs of std::pin.

Unpin requirement makes it hard to set timeout for async fn.

yoshuawuyts commented 5 years ago

@uHOOCCOOHu I have to admit I'm not as comfortable with pin semantics as I wish I was. If you suspect this can be removed & it makes life easier, we'd definitely welcome a patch to do so!

We definitely care a lot about allowing async fn timeouts! (:

oxalica commented 5 years ago

I'm quite sure that F: Unpin is not required for time::Timeout (and so do TimeoutAsyncRead and TimeoutStream), so the bound in time::FutureExt can be also removed.

Sadly, if so, the poll() method seems impossible to write without unsafe (required for pinning projection). Since we have #![deny(unsafe_code)], I'm not sure if it is acceptable.

sdroege commented 5 years ago

Until there are more Pin helper functions, I don't think #![deny(unsafe_code)] is achievable for any project implementing futures manually unless you box everything and require Unpin on everything.