From b18c7d85a92011bcc094b33e6a3c646a3cfca8f3 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 22 Jul 2024 17:34:29 +0100 Subject: [PATCH 1/3] Docs for Waker and LocalWaker: Add cross-refs in comment --- library/core/src/task/wake.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/core/src/task/wake.rs b/library/core/src/task/wake.rs index 86a965f68e0..4582bf9f422 100644 --- a/library/core/src/task/wake.rs +++ b/library/core/src/task/wake.rs @@ -531,6 +531,10 @@ pub fn will_wake(&self, other: &Waker) -> bool { /// Returns a reference to a `Waker` that does nothing when used. /// + // Note! Much of the documentation for this method is duplicated + // in the docs for `LocalWaker::noop`. + // If you edit it, consider editing the other copy too. + // /// This is mostly useful for writing tests that need a [`Context`] to poll /// some futures, but are not expecting those futures to wake the waker or /// do not need to do anything specific if it happens. @@ -784,6 +788,10 @@ pub fn will_wake(&self, other: &LocalWaker) -> bool { /// Creates a new `LocalWaker` that does nothing when `wake` is called. /// + // Note! Much of the documentation for this method is duplicated + // in the docs for `Waker::noop`. + // If you edit it, consider editing the other copy too. + // /// This is mostly useful for writing tests that need a [`Context`] to poll /// some futures, but are not expecting those futures to wake the waker or /// do not need to do anything specific if it happens. From c404406a87412d50604893d8c347744195ce2dea Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 22 Jul 2024 17:35:11 +0100 Subject: [PATCH 2/3] LocalWaker docs: Make long-ago omitted but probably intended changes In 6f8a944ba4311cbcf5922132721095c226c6fbab, titled Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`. the summary line for Waker was changed: - /// Creates a new `Waker` that does nothing when `wake` is called. + /// Returns a reference to a `Waker` that does nothing when used. and the sentence about clone was added. LocalWaker's docs were not changed, even though the types were, but there is no explanation for why not. It seems like it was simply a slip induced by the clone-and-hack. --- library/core/src/task/wake.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/core/src/task/wake.rs b/library/core/src/task/wake.rs index 4582bf9f422..baa8a956f8a 100644 --- a/library/core/src/task/wake.rs +++ b/library/core/src/task/wake.rs @@ -786,7 +786,7 @@ pub fn will_wake(&self, other: &LocalWaker) -> bool { Self { waker } } - /// Creates a new `LocalWaker` that does nothing when `wake` is called. + /// Returns a reference to a `LocalWaker` that does nothing when used. /// // Note! Much of the documentation for this method is duplicated // in the docs for `Waker::noop`. @@ -796,6 +796,8 @@ pub fn will_wake(&self, other: &LocalWaker) -> bool { /// some futures, but are not expecting those futures to wake the waker or /// do not need to do anything specific if it happens. /// + /// If an owned `LocalWaker` is needed, `clone()` this one. + /// /// # Examples /// /// ``` From 9a95573c2bb3e76643f0c1c4deb5eea24c37ec5c Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 5 Aug 2024 17:25:00 +0100 Subject: [PATCH 3/3] Add cautionary paragraph about noop wakers. Based on a suggestion from @kpreid, with some further editing. --- library/core/src/task/wake.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/core/src/task/wake.rs b/library/core/src/task/wake.rs index baa8a956f8a..4d2de6be025 100644 --- a/library/core/src/task/wake.rs +++ b/library/core/src/task/wake.rs @@ -539,6 +539,10 @@ pub fn will_wake(&self, other: &Waker) -> bool { /// some futures, but are not expecting those futures to wake the waker or /// do not need to do anything specific if it happens. /// + /// More generally, using `Waker::noop()` to poll a future + /// means discarding the notification of when the future should be polled again. + /// So it should only be used when such a notification will not be needed to make progress. + /// /// If an owned `Waker` is needed, `clone()` this one. /// /// # Examples @@ -796,6 +800,10 @@ pub fn will_wake(&self, other: &LocalWaker) -> bool { /// some futures, but are not expecting those futures to wake the waker or /// do not need to do anything specific if it happens. /// + /// More generally, using `LocalWaker::noop()` to poll a future + /// means discarding the notification of when the future should be polled again, + /// So it should only be used when such a notification will not be needed to make progress. + /// /// If an owned `LocalWaker` is needed, `clone()` this one. /// /// # Examples