Replace unstable Waker::noop() with Waker::NOOP.

[kpreid]

As discussed on Zulip, across std, outside of argumentless new() constructor functions, stable constant values are generally provided using const items rather than const fns. Therefore, this change is more consistent API design.

Further, WG-async writes:

WG-async is in favor of this being an associated constant of type &Waker.

This PR implements that decision.

[rustbot]

r? @cuviper

rustbot has assigned @cuviper.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[kpreid]

🤦 Didn't do the find and replace everywhere. I thought this was a suspiciously small diff…

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in coverage tests.

cc @Zalathar

[RalfJung]

FWIW, since constants get promoted, it'd now also be possible to change this back to have type Waker and use it as

let ctx = &mut Context::from_waker(&Waker::NOOP);

This would be basically un-doing #119984. The let that motivated #119984 are no longer needed.

[kpreid]

I already tried that initially and the compiler rejects it.

I prepared a change which replaced Waker::noop() with Waker::NOOP. Unfortunately, it seems that this doesn't enable &NOOP usage:

let mut cx = &mut Context::from_waker(&Waker::NOOP);
cx.waker().wake_by_ref();

error[E0716]: temporary value dropped while borrowed
   --> src/wake.rs:404:44
    |
404 |     let mut cx = &mut Context::from_waker(&Waker::NOOP);
    |                                            ^^^^^^^^^^^ - temporary value is freed at the end of this statement

If I understand correctly now, this is not possible because the Waker has a destructor. Therefore, since use via constant promotion is not possible, it seems wise to offer a &Waker value, in which case making it a constant rather than a const fn doesn't provide any advantages (but still might be a choice of API aesthetics).

[RalfJung]

Ah, true, if it has a destructor it cannot get promoted. Good point.

[joboet]

I'm strongly opposed to this. Constants are used when we actually care about the specific value in question (so you could e.g. match on it), not just about its semantics. Hence e.g. ONCE_INIT was deprecated in favour of Once::new(). Here, we don't care about the specific vtable or pointer used, and wouldn't care if they changed across calls to Waker::noop, as long as the resulting Waker obeyed the rules outlined by the documentation.

[kpreid]

I'm strongly opposed to this.

My own interest in this matter is that I want a stable noop waker, and in service of that, I'm trying to put work into making it more usable (-> &Waker) and consistent/rule-following (whatever that turns out to mean) API that's fit for stabilization. So I have no strong opinion on whether it should be a constant or a function, but WG-async already concluded that it should be a constant, so I'd like to ask that you or T-libs-api get in touch with WG-async and resolve this conflict yourselves.

I do appreciate your point that constants imply matchability/equality that is irrelevant here.

[cuviper]

r? libs-api

[jhpratt]

@bors r+

[bors]

📌 Commit 25c9f50 has been approved by jhpratt

It is now in the queue for this repository.

[cuviper]

@jhpratt are you overriding @joboet's objection?

I think T-libs-api needs to resolve this.

[jhpratt]

Ah, right, it was a working group, not a team that gave the go-ahead. My bad.

@bors r-

[traviscross]

Constants are used when we actually care about the specific value in question (so you could e.g. match on it), not just about its semantics.

Curious, do we happen to have a policy or guideline written up anywhere on this?

Here, we don't care about the specific vtable or pointer used, and wouldn't care if they changed across calls to Waker::noop, as long as the resulting Waker obeyed the rules outlined by the documentation.

Is this true in this case? Could it not be useful to have a canonical NOOP waker and to be able to check whether one has such a no-op waker, e.g. so as to enable taking a fast-path?

[RalfJung]

Could it not be useful to have a canonical NOOP waker and to be able to check whether one has such a no-op waker, e.g. so as to enable taking a fast-path?

That can't be achieved with a const though, at least not reliably. It needs a static.

And for various reasons (most of them related to pattern matching), consts currently cannot point to statics.

[kpreid]

That seems like potentially a good reason to not make a const NOOP, but rather to keep noop() to preserve the option of adding a static NOOP later (whereas const NOOP → static NOOP would be a breaking change).

[traviscross]

That seems like potentially a good reason to not make a const NOOP, but rather to keep noop() to preserve the option of adding a static NOOP later (whereas const NOOP → static NOOP would be a breaking change).

Agreed (speaking personally).

Regarding WG-async, nobody had particularly strong feelings about whether this should be a const or a const fn. We were persuaded by the consistency argument here. But if it's more consistent or otherwise advantageous to make this a const fn instead, my own estimate is that the group would also be happy with that. We'll pick this back up for discussion to confirm.

[kpreid]

Could it not be useful to have a canonical NOOP waker and to be able to check whether one has such a no-op waker, e.g. so as to enable taking a fast-path?

Regardless of how the API for getting a noop waker turns out,

• You can ask this question using Waker::NOOP.will_wake(other_waker), iff std offers a guarantee that the addresses involved are stable, but not if it doesn't. This guarantee can be a matter of documentation, not constraining the Rust-item-schema part of the API design.
• You can't ask this question by comparing or pattern-matching the waker, because Waker isn't PartialEq, let alone StructuralPartialEq.

---

Based on discussion with T-libs-api, I am currently inclined to close this PR and propose no other API changes — there is no use case any of us know of that would be improved by making the noop waker available as a const or a static, but if some is discovered later, it can be added later.

One possibility that was raised was stabilizing Waker::noop() as a non-constant would make it easier to offer the abovementioned will_wake() comparison guarantee using the current language capabilities (by returning a reference to a static, which is currently not allowed in const fn). But I don't think that that is sufficiently valuable to delay stabilization for; in particular, the noop-waker use cases I am familiar with are (1) "I am going to poll this future exactly once in its life" and (2) "I am going to poll this future on a fixed schedule even if it is unnecessary", and it seems unlikely to me that the overhead of stashing a useless waker is significant in those cases.

[dtolnay]

In that case we'll move forward on stabilizing Waker::noop() as is. Thanks!