Update to new futures_api

[taiki-e]

This PR covers changes by rust-lang/rust#59119 and rust-lang/rust#59733.

cc #1518 (I initially intended to close it with this PR, but in fact it might be better to close it when the next version is released.)

[yoshuawuyts]

Is there an ETA when we could land this? The changes have landed in the latest nightly, and currently a good amount of packages are broken until we can land this.

[taiki-e]

(Landing it now does not mean that we can immediately release the next version.)

[taiki-e]

Also, rust-lang/rust#59733 is already approved, so It will break again in a few days.

[davidbarsky]

Possibly a ignorant question, but would this fix land (pending time/availability from people, naturally) when/if rust-lang/rust@df25d79 is part of a nightly release?

[taiki-e]

@cramertj @Nemo157 Could you review this PR?

Considering FCP takes more than a week, I think that it is preferable to land this at this timing.

[najamelan]

Updating to nightly today still gave me another error:

error[E0507]: cannot move out of borrowed content
   --> /futures-rs/futures-core/src/task/__internal/atomic_waker.rs:244:17
    |
244 |                 waker.wake();
    |                 ^^^^^ cannot move out of borrowed content

Not entirely sure that this should be dealt with in this pull request, but I think it's because of the waker by ref last minute changes before stabelization. Cloning it here was my fast braindead fix, but I don't know whether that's the best way to solve this.

[taiki-e]

@najamelan See 265ca12.

[najamelan]

@taiki-e Oh, sorry, you're awesome. I had missed the extra commit... compiles fine. Thanks

[Nemo157]

Looks great, thanks for updating this all.

Everything except the one comment I've left looks like a straight-forward update of the API, there's potentially some performance optimization that could be done using the by-value waking, but that would be best as a separate PR.

[Nemo157]

I'm not certain whether it makes sense to keep the separation of waking by-value and by-ref for ArcWake, it looks like every implementation of ArcWake::wake delegates to ArcWake::wake_by_ref. But maybe the thread pool implementation can be updated to take advantage of it.

I think it makes more sense to provide the ArcWake::wake implementation as delegating to ArcWake::wake_by_ref, that would allow dropping all the current delegating implementations, and means that implementors wouldn't accidentally cause an unnecessary clone by only implementing ArcWake::wake (they may still cause an unnecessary clone by only implementing ArcWake::wake_by_ref and not taking advantage of being able to reuse the Arc in ArcWake::wake, but that seems like a more obvious performance issue since the clone will be happening in their code instead of the provided method).

[taiki-e]

I think it makes more sense to provide the ArcWake::wake implementation as delegating to ArcWake::wake_by_ref, that would allow dropping all the current delegating implementations, and means that implementors wouldn't accidentally cause an unnecessary clone by only implementing ArcWake::wake (they may still cause an unnecessary clone by only implementing ArcWake::wake_by_ref and not taking advantage of being able to reuse the Arc in ArcWake::wake, but that seems like a more obvious performance issue since the clone will be happening in their code instead of the provided method).

Thanks for pointing that out, that makes a lot of sense.

[Nemo157]

(for others that want to start experimenting off this, here's a block you can copy-paste into your Cargo.toml to set all the overrides):

[patch.crates-io]
futures-preview = { git = "https://github.com/taiki-e/futures-rs", branch = "futures_api" }
futures-core-preview = { git = "https://github.com/taiki-e/futures-rs", branch = "futures_api" }
futures-util-preview = { git = "https://github.com/taiki-e/futures-rs", branch = "futures_api" }
futures-sink-preview = { git = "https://github.com/taiki-e/futures-rs", branch = "futures_api" }
futures-channel-preview = { git = "https://github.com/taiki-e/futures-rs", branch = "futures_api" }
futures-executor-preview = { git = "https://github.com/taiki-e/futures-rs", branch = "futures_api" }
futures-io-preview = { git = "https://github.com/taiki-e/futures-rs", branch = "futures_api" }
futures-test-preview = { git = "https://github.com/taiki-e/futures-rs", branch = "futures_api" }

[Nemo157]

Thanks again ❤️, hopefully we can get a release out soon with these fixes in (maybe even one worth promoting to beta).

[cramertj]

Thanks for working to get this landed! All the help is extremely appreciated. I'll get a new release out ASAP.

[vorot93]

@cramertj Could you please take a look at #1524 at first? google/tarpc may miss the new release otherwise.

[cramertj]

@vorot93 merged! :) (In the future, cc'ing me or adding me as a reviewer will get my attention quicker, though I try to scan the repo for PRs every once in a while)