From c4fdac9fe60bcfc8ca35c3997987d7cac16c4d6c 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 --- core/src/task/wake.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/src/task/wake.rs b/core/src/task/wake.rs index 86a965f68e085..4582bf9f42245 100644 --- a/core/src/task/wake.rs +++ b/core/src/task/wake.rs @@ -531,6 +531,10 @@ impl Waker { /// 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 @@ impl LocalWaker { /// 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 971aa37f27b5e5ecbb952c7037ee021f18c23185 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. --- core/src/task/wake.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/task/wake.rs b/core/src/task/wake.rs index 4582bf9f42245..baa8a956f8af1 100644 --- a/core/src/task/wake.rs +++ b/core/src/task/wake.rs @@ -786,7 +786,7 @@ impl LocalWaker { 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 @@ impl LocalWaker { /// 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 0497f0c6c915799e80f05f95330c2ab6756d3420 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. --- core/src/task/wake.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/src/task/wake.rs b/core/src/task/wake.rs index baa8a956f8af1..4d2de6be025bc 100644 --- a/core/src/task/wake.rs +++ b/core/src/task/wake.rs @@ -539,6 +539,10 @@ impl Waker { /// 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 @@ impl LocalWaker { /// 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