Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking Issue for task::Waker::noop #98286

Open
1 of 3 tasks
SabrinaJewson opened this issue Jun 20, 2022 · 24 comments
Open
1 of 3 tasks

Tracking Issue for task::Waker::noop #98286

SabrinaJewson opened this issue Jun 20, 2022 · 24 comments
Labels
AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. I-async-nominated Nominated for discussion during an async working group meeting. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. WG-async Working group: Async & await

Comments

@SabrinaJewson
Copy link
Contributor

SabrinaJewson commented Jun 20, 2022

Feature gate: #![feature(noop_waker)]

This is a tracking issue for task::Waker::noop, a way to easily crate task::Wakers that do nothing.

Public API

// core::task

impl Waker {
    #[must_use]
    pub const fn noop() -> &Waker { /* … */ }
}

Steps / History

Unresolved Questions

  • Should we also add RawWaker::noop()? (I don't think so, I can't think of a use case for it)
  • Should we also add Context::noop()? Depending on the direction Context goes a “noop context” might not even make sense in future.
  • Should it be an associated constant instead? That would allow for let cx = &mut Context::from_waker(&Waker::NOOP); to work on one line (without additional local variables) which is convenient.
    • The return type of Waker::noop() has now been changed to &Waker.
@SabrinaJewson SabrinaJewson added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 20, 2022
@JarredAllen
Copy link
Contributor

+1 on this feature, I find myself re-implementing a noop Waker frequently.

As for the third question, I'd personally prefer either an associated constant or changing the return signature to fn noop() -> &'static Waker;, because the only use I have for it involves immediately shoving it into Context::from_waker, which is more convenient with either of those signatures, though I'm not sure if other people have other use cases that are made easier by a function returning an owned value.

bors added a commit to rust-lang-ci/rust that referenced this issue Jun 7, 2023
Add `task::Waker::noop`

I have found myself reimplementing this function many times when I need a `Context` but don't have a runtime or `futures` to hand.

Prior art: [`futures::task::noop_waker`](https://docs.rs/futures/0.3/futures/task/fn.noop_waker.html) and [`futures::task::noop_waker_ref`](https://docs.rs/futures/0.3/futures/task/fn.noop_waker_ref.html)

Tracking issue: rust-lang#98286

Unresolved questions:
1. Should we also add `RawWaker::noop()`? (I don't think so, I can't think of a use case for it)
2. Should we also add `Context::noop()`? Depending on the future direction `Context` goes a "noop context" might not even make sense in future.
3. Should it be an associated constant instead? That would allow for `let cx = &mut Context::from_waker(&Waker::NOOP);` to work on one line which is pretty nice. I don't really know what the guideline is here.

r? rust-lang/libs-api `@rustbot` label +T-libs-api -T-libs
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Aug 24, 2023
Add `task::Waker::noop`

I have found myself reimplementing this function many times when I need a `Context` but don't have a runtime or `futures` to hand.

Prior art: [`futures::task::noop_waker`](https://docs.rs/futures/0.3/futures/task/fn.noop_waker.html) and [`futures::task::noop_waker_ref`](https://docs.rs/futures/0.3/futures/task/fn.noop_waker_ref.html)

Tracking issue: rust-lang/rust#98286

Unresolved questions:
1. Should we also add `RawWaker::noop()`? (I don't think so, I can't think of a use case for it)
2. Should we also add `Context::noop()`? Depending on the future direction `Context` goes a "noop context" might not even make sense in future.
3. Should it be an associated constant instead? That would allow for `let cx = &mut Context::from_waker(&Waker::NOOP);` to work on one line which is pretty nice. I don't really know what the guideline is here.

r? rust-lang/libs-api `@rustbot` label +T-libs-api -T-libs
@traviscross traviscross added the WG-async Working group: Async & await label Dec 4, 2023
@traviscross
Copy link
Contributor

@rustbot labels +I-async-nominated

Let's discuss this in a WG-async meeting as this has come up in the context of #101196.

@rustbot rustbot added the I-async-nominated Nominated for discussion during an async working group meeting. label Dec 4, 2023
@yoshuawuyts
Copy link
Member

yoshuawuyts commented Dec 18, 2023

Speaking just for myself here: to me this seems generally useful as a utility, somewhat akin to future::pending and iter::empty. But I wonder whether instead of exposing it as a method on Waker, it might make more sense to create a new free function for it? That would place it a bit off the hot path, and more in the utilities section.

This would basically be a shorthand for creating a waker from Arc via the Wake trait, but minus a bit of overhead when cloning it.

@traviscross traviscross added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Dec 18, 2023
@traviscross
Copy link
Contributor

We discussed this in the WG-async triage call today. While everyone did see use cases for this, some also had concerns about this potentially being misused. E.g. someone might use this when the person should instead call .await. No-one seemed to think that was a blocker by itself ("Rust has lots of hard things"). Some were also unsure whether this was worth it and whether the cost of duplicating this code might actually induce a useful friction.

We were curious to hear more about the use cases. If people here could help to fill in the details of the various use cases for this, that would help with discussion, and it may help enable a WG-async consensus to form.

@Ralith
Copy link
Contributor

Ralith commented Dec 19, 2023

A use case today, from quinn: I want to send a UDP datagram if doing so would succeed immediately, or otherwise drop the datagram entirely, because it's a response to a cheap request I know the peer will retry. It'd be convenient to be able to express this through a normal async UDP socket interface.

@kpreid
Copy link
Contributor

kpreid commented Dec 19, 2023

Two use cases that I've come across:

  • Running a Future concurrently with some other, much heavier, loop (where the output of the future would tell it to stop or otherwise give it relevant input, or the future needs to always be run in some particular context or with particular timing relative to the other work), where polling the Future unnecessarily is insignificant relative to the other work being done, so it isn't worth bothering implementing a wakeup.
    • But in this scenario, it might be better to provide a generic Future wrapper which internally provides a Waker in order to exit early if polled early, and itself presents poll_without_a_waker() to its owner.
  • Running a Future, starting from a synchronous context, that we know won't actually return Pending, because it came from generic async code but contains no leaf futures that suspend.
    • This can be done instead with a block_on(), but that requires more code than needed bringing in synchronization primitives until such time as we get LocalWaker, and it could hide bugs where the future does suspend and we'd rather panic about it.
    • But now_or_never() is a simpler to use, narrower function for that. Notably, it doesn't require pinning the future.

But I've reached for futures::task::noop_waker_ref() more times than that, for reasons I don't recall in detail — probably experiments and code that eventually got replaced. So, even though I just listed a bunch of alternatives, I think there should be a noop Waker readily available, because there's a bunch of things you might want to do quickly and then later replace with something more refined.


Separately: I notice that one of the unresolved questions is:

Should it be an associated constant instead? That would allow for let cx = &mut Context::from_waker(&Waker::NOOP); to work on one line (without additional local variables) which is convenient.

I think that either this, or providing the equivalent of noop_waker_ref(), is a good idea. Either way you can always get an owned waker with .clone(), but it's not so convenient to create a reference given an owned waker. (And someone might come up with a use for a Context<'static>, which can't be made from an owned value without Box::leak().)

And in 100% of the cases I've ever used a noop waker, I used noop_waker_ref(), not the owned version noop_waker(). (But I haven't written any future combinators or other code that I imagine might be interested might be want to use an owned noop waker as a placeholder value in a data structure, as opposed to immediately creating a Context which only wants a reference.)

@jkarneges
Copy link
Contributor

+1 for something that can provide a reference.

For me, a noop waker is mainly interesting for tests or experimentation/playground, and enabling another line of code to be saved seems within the spirit of that. I don't know if I've ever needed an owned noop waker, and as kpreid suggests can just clone() if that ever happens.

No opinion on constant vs function.

kpreid added a commit to kpreid/rust that referenced this issue Jan 14, 2024
The advantage of this is that `&Waker::NOOP` can live for '`static`,
so it does not need to be assigned to a variable to be used in a
`Context` creation, which is, as shown in the changes to the tests in
this commit, is the most common thing to want to do with a noop waker.

An alternative would be to make it be of type `&'static Waker`,
which would make that common case even shorter, but that would mean that
if an owned `Waker` is desired it must be `.clone()`d from the constant.
Or, both versions could be provided, like `futures::task::noop_waker()`
and `futures::task::noop_waker_ref()`. But either option seems less
elegant to me.

Previous discussion on the tracking issue starting here:
rust-lang#98286 (comment)
@kpreid
Copy link
Contributor

kpreid commented Jan 15, 2024

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).

kpreid added a commit to kpreid/rust that referenced this issue Jan 15, 2024
The advantage of this is that it does not need to be assigned to a
variable to be used in a `Context` creation, which is the most common
thing to want to do with a noop waker.

If an owned noop waker is desired, it can be created by cloning, but the
reverse is harder. Alternatively, both versions could be provided, like
`futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but
that seems to me to be API clutter for a very small benefit, whereas
having the `&'static` reference available is a large benefit.

Previous discussion on the tracking issue starting here:
rust-lang#98286 (comment)
@traviscross
Copy link
Contributor

In the WG-async call today, we discussed this, resulting in the following consensus:

Consensus: WG-async is in favor of returning &Waker from the function (as in #119984) due to ergonomic considerations related to const promotion and temporary lifetimes. We're OK with the Waker::noop() name for this function since we expect this is what people will need most of the time, and if we later did find that people needed an owned no-op waker (for reasons we can't imagine today) that it could receive the longer name, e.g. Waker::noop_owned. In our discussions, we did wonder about whether it would also make sense to add Context::noop() (which would return an owned Context) which would shorten the most common use-case further.

@kpreid
Copy link
Contributor

kpreid commented Jan 15, 2024

@traviscross Does WG-async have any opinion about the question of making it be Waker::NOOP, a constant instead of a function? Per this Zulip discussion it's very rare across std for constant values to be provided as functions rather than constants (except for new() functions).

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 16, 2024
Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.

The advantage of this is that it does not need to be assigned to a variable to be used in a `Context` creation, which is the most common thing to want to do with a noop waker. It also avoids unnecessarily executing the dynamically dispatched drop function when the noop waker is dropped.

If an owned noop waker is desired, it can be created by cloning, but the reverse is harder to do since it requires declaring a constant. Alternatively, both versions could be provided, like `futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but that seems to me to be API clutter for a very small benefit, whereas having the `&'static` reference available is a large reduction in boilerplate.

[Previous discussion on the tracking issue starting here](rust-lang#98286 (comment))
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 16, 2024
Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.

The advantage of this is that it does not need to be assigned to a variable to be used in a `Context` creation, which is the most common thing to want to do with a noop waker. It also avoids unnecessarily executing the dynamically dispatched drop function when the noop waker is dropped.

If an owned noop waker is desired, it can be created by cloning, but the reverse is harder to do since it requires declaring a constant. Alternatively, both versions could be provided, like `futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but that seems to me to be API clutter for a very small benefit, whereas having the `&'static` reference available is a large reduction in boilerplate.

[Previous discussion on the tracking issue starting here](rust-lang#98286 (comment))
compiler-errors added a commit to compiler-errors/rust that referenced this issue Jan 17, 2024
Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.

The advantage of this is that it does not need to be assigned to a variable to be used in a `Context` creation, which is the most common thing to want to do with a noop waker. It also avoids unnecessarily executing the dynamically dispatched drop function when the noop waker is dropped.

If an owned noop waker is desired, it can be created by cloning, but the reverse is harder to do since it requires declaring a constant. Alternatively, both versions could be provided, like `futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but that seems to me to be API clutter for a very small benefit, whereas having the `&'static` reference available is a large reduction in boilerplate.

[Previous discussion on the tracking issue starting here](rust-lang#98286 (comment))
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 17, 2024
Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.

The advantage of this is that it does not need to be assigned to a variable to be used in a `Context` creation, which is the most common thing to want to do with a noop waker. It also avoids unnecessarily executing the dynamically dispatched drop function when the noop waker is dropped.

If an owned noop waker is desired, it can be created by cloning, but the reverse is harder to do since it requires declaring a constant. Alternatively, both versions could be provided, like `futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but that seems to me to be API clutter for a very small benefit, whereas having the `&'static` reference available is a large reduction in boilerplate.

[Previous discussion on the tracking issue starting here](rust-lang#98286 (comment))
compiler-errors added a commit to compiler-errors/rust that referenced this issue Jan 17, 2024
Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.

The advantage of this is that it does not need to be assigned to a variable to be used in a `Context` creation, which is the most common thing to want to do with a noop waker. It also avoids unnecessarily executing the dynamically dispatched drop function when the noop waker is dropped.

If an owned noop waker is desired, it can be created by cloning, but the reverse is harder to do since it requires declaring a constant. Alternatively, both versions could be provided, like `futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but that seems to me to be API clutter for a very small benefit, whereas having the `&'static` reference available is a large reduction in boilerplate.

[Previous discussion on the tracking issue starting here](rust-lang#98286 (comment))
kpreid added a commit to kpreid/rust that referenced this issue Jan 17, 2024
The advantage of this is that it does not need to be assigned to a
variable to be used in a `Context` creation, which is the most common
thing to want to do with a noop waker.

If an owned noop waker is desired, it can be created by cloning, but the
reverse is harder. Alternatively, both versions could be provided, like
`futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but
that seems to me to be API clutter for a very small benefit, whereas
having the `&'static` reference available is a large benefit.

Previous discussion on the tracking issue starting here:
rust-lang#98286 (comment)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 19, 2024
Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.

The advantage of this is that it does not need to be assigned to a variable to be used in a `Context` creation, which is the most common thing to want to do with a noop waker. It also avoids unnecessarily executing the dynamically dispatched drop function when the noop waker is dropped.

If an owned noop waker is desired, it can be created by cloning, but the reverse is harder to do since it requires declaring a constant. Alternatively, both versions could be provided, like `futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but that seems to me to be API clutter for a very small benefit, whereas having the `&'static` reference available is a large reduction in boilerplate.

[Previous discussion on the tracking issue starting here](rust-lang#98286 (comment))
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 20, 2024
Rollup merge of rust-lang#119984 - kpreid:waker-noop, r=dtolnay

Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.

The advantage of this is that it does not need to be assigned to a variable to be used in a `Context` creation, which is the most common thing to want to do with a noop waker. It also avoids unnecessarily executing the dynamically dispatched drop function when the noop waker is dropped.

If an owned noop waker is desired, it can be created by cloning, but the reverse is harder to do since it requires declaring a constant. Alternatively, both versions could be provided, like `futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but that seems to me to be API clutter for a very small benefit, whereas having the `&'static` reference available is a large reduction in boilerplate.

[Previous discussion on the tracking issue starting here](rust-lang#98286 (comment))
@kpreid
Copy link
Contributor

kpreid commented Jan 20, 2024

Update: The return type of Waker::noop() has been changed to be &Waker through my PR #119984. That means that the rationale stated in the unresolved question

  • Should it be an associated constant instead? That would allow for let cx = &mut Context::from_waker(&Waker::NOOP); to work on one line (without additional local variables) which is convenient.

is now moot, but there is still the question of style (as per my previous comment).

github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jan 21, 2024
Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.

The advantage of this is that it does not need to be assigned to a variable to be used in a `Context` creation, which is the most common thing to want to do with a noop waker. It also avoids unnecessarily executing the dynamically dispatched drop function when the noop waker is dropped.

If an owned noop waker is desired, it can be created by cloning, but the reverse is harder to do since it requires declaring a constant. Alternatively, both versions could be provided, like `futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but that seems to me to be API clutter for a very small benefit, whereas having the `&'static` reference available is a large reduction in boilerplate.

[Previous discussion on the tracking issue starting here](rust-lang/rust#98286 (comment))
@traviscross
Copy link
Contributor

@kpreid: You raise a good question about whether this should instead be a const. We'll be sure to discuss that at the next WG-async triage meeting.

@traviscross
Copy link
Contributor

We discussed this in the WG-async call today. Our consensus was:

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

Thanks for @kpreid for raising this point.

As far as why it should be of type &Waker rather than Waker, it's due to code such as this:

pub const NOP_CONTEXT: Context<'static> =
    Context::from_waker(&Waker::NOOP);
//~^ ERROR destructor of `Waker` cannot be evaluated at compile-time
//~| NOTE the destructor for this type cannot be evaluated in constants
//~^ ERROR temporary value dropped while borrowed
//~| creates a temporary value which is freed while still in use

If the constant has type &Waker, then that code works.

kpreid added a commit to kpreid/rust that referenced this issue Mar 23, 2024
As discussed in
<https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Associated.20constants.20vs.2E.20functions.20in.20std.20API>,
across `std, outside of argumentless `new()` constructor functions,
stable constant values are generally provided using `const` items rather
than `const fn`s. Therefore, this change is more consistent API design.

WG-async approves of this change, per
<rust-lang#98286 (comment)>.
kpreid added a commit to kpreid/rust that referenced this issue Mar 23, 2024
As discussed in
<https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Associated.20constants.20vs.2E.20functions.20in.20std.20API>,
across `std`, outside of argumentless `new()` constructor functions,
stable constant values are generally provided using `const` items rather
than `const fn`s. Therefore, this change is more consistent API design.

WG-async approves of making this change, per
<rust-lang#98286 (comment)>.
kpreid added a commit to kpreid/rust that referenced this issue Mar 23, 2024
As discussed in
<https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Associated.20constants.20vs.2E.20functions.20in.20std.20API>,
across `std`, outside of argumentless `new()` constructor functions,
stable constant values are generally provided using `const` items rather
than `const fn`s. Therefore, this change is more consistent API design.

WG-async approves of making this change, per
<rust-lang#98286 (comment)>.
@kpreid
Copy link
Contributor

kpreid commented Mar 23, 2024

For cross-reference, and @traviscross FYI, I opened #122924 to implement the change to a constant, and I got an objection from @joboet on the grounds that

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.

@traviscross
Copy link
Contributor

traviscross commented Mar 25, 2024

Replied to that here with further discussion continuing there.

@kpreid
Copy link
Contributor

kpreid commented Apr 2, 2024

T-libs-api and myself discussed the question of Waker::noop() vs. Waker::NOOP. One of the things that we noticed is that there is no value in providing a constant for comparison purposes, because Waker isn't PartialEq, let alone StructuralPartialEq. Rather, it provides the will_wake() function, which is equally applicable regardless of what API is chosen for the noop waker. So, I'm personally not going to pursue changing it to a constant NOOP, and would be happy to see this stabilized in its current form: const fn noop() -> &'static Waker. (More commentary on the PR.)

@jkarneges
Copy link
Contributor

to me this seems generally useful as a utility, somewhat akin to future::pending and iter::empty. But I wonder whether instead of exposing it as a method on Waker, it might make more sense to create a new free function for it?

Apologies for further bikeshedding on this smallest of features, but was this form ever considered? I kind of like std::task::noop_waker, as it's probably what would have been proposed if Waker were a trait, and Waker is pretty much a trait in spirit.

Either way, what's left? FCP?

@JoJoJet
Copy link
Contributor

JoJoJet commented Jul 20, 2024

Looking forward to this feature. I just noticed that the documentation for Waker::will_wake asserts that two wakers are guaranteed to wake the same task if will_wake returns true. I think that this function violates that assertion which should probably be considered

I think that the current behavior of will_wake is probably reasonable but it might be good to rephrase that guarantee

@kpreid
Copy link
Contributor

kpreid commented Jul 20, 2024

The thing is that the entire concept of “task” is not formally defined (given various possible executors and combinators; e.g. Shared causes a future to potentially be polled by many different parent tasks; is that itself a task?) and irrelevant to implementing a Future. The only thing that actually matters is that a Future eventually signals its Waker when it is able to make progress, and the Future will be eventually polled or dropped after it signals its Waker. So, relying on anything about “task” identity is a bad idea whether or not Waker::noop() exists.

“Task” is basically “ownership of a Future being driven to completion”, and similarly to ordinary ownership, you can't point at any single type or language construct and say “this is an owner” or “this is a task”; you can only say:

  • This value should be dropped when it is no longer in use, and no sooner than that.
    How is that implemented? That's the owner.
  • This Future should be polled when it wakes, and ideally not when it doesn't.
    How is that implemented? That's the task.

In this frame, Waker::noop() is a degenerate case of “task” in much the same way as Box::leak() is a degenerate case of ownership: there are no entities in the program which are playing the role of task or owner, but that doesn't matter because the necessary rules are followed anyway. We could say that polling with Waker::noop() makes the future part of “the noop task”, if we like; there's nothing concrete to point at to contradict the existence of such a task.

@JoJoJet
Copy link
Contributor

JoJoJet commented Jul 20, 2024

So, relying on anything about “task” identity is a bad idea whether or not Waker::noop() exists.

I don't necessarily disagree, but the whole point of will_wake is to let you reason about task identity. I don't think this is a major problem, but for correctness sake it would make sense to weaken that guarantee, since it doesn't really guarantee anything. will_wake is already described as a "best effort", so it would be good to document that there are degenerate cases where the concept of task identity falls apart

@kpreid
Copy link
Contributor

kpreid commented Jul 20, 2024

I don't necessarily disagree, but the whole point of will_wake is to let you reason about task identity.

As I see it, the whole point of will_wake is to let you short-circuit some mutation and cloning if you already have an acceptable waker. But, we should probably not continue having this argument on a tracking issue, as it clutters the thread with things that aren't status updates on Waker::noop(), and notifies people who don't care. Perhaps you could file a separate issue for this documentation/semantics concern? (I'm presuming it's unlikely to imply any changes to what Waker::noop() actually does or is named.)

@JoJoJet
Copy link
Contributor

JoJoJet commented Jul 20, 2024

I'm not arguing, just sharing concerns :)

@ijackson
Copy link
Contributor

I wanted a no-op waker for an exciting future I'm writing. I ended up implementing my own, of course.

I also noticed the possibility for misuse, and there was some confusion amongst some of my colleagues. I thought it best to document when not to use this, and made #128064 for that.

It would be nice to see this stabilised in some form, even if only to give those docs more exposure.

@c410-f3r
Copy link
Contributor

c410-f3r commented Sep 1, 2024

#128064 was merged so can we proceed with the stabilization of Waker::noop? AFAICT, the addition of RawWaker::noop() or Context::noop() is orthogonal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. I-async-nominated Nominated for discussion during an async working group meeting. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. WG-async Working group: Async & await
Projects
None yet
Development

No branches or pull requests