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

Docs prose say Sleep doesn't implement Unpin, but sidebar does #5901

Closed
jrose-signal opened this issue Aug 1, 2023 · 5 comments · Fixed by #5916
Closed

Docs prose say Sleep doesn't implement Unpin, but sidebar does #5901

jrose-signal opened this issue Aug 1, 2023 · 5 comments · Fixed by #5916
Labels
A-tokio Area: The main tokio crate M-time Module: tokio/time

Comments

@jrose-signal
Copy link

jrose-signal commented Aug 1, 2023

Since v1.1.0, Sleep has been documented as follows:

This type does not implement the Unpin trait, which means that if you use it with select! or by calling poll, you have to pin it first. If you use it with .await, this does not apply.

But the autogenerated sidebar says it does implement Unpin, since v1.0.1. I did find #3267 and #3278 and even #3278 (comment):

We should have a test that asserts that Sleep is !Unpin (same for other types that contain pinned Sleeps).

And yet Sleep is Unpin. I tested locally incorrectly and thought Sleep was actually Unpin, but it's just the autogenerated docs that are confusing.

@jrose-signal jrose-signal added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Aug 1, 2023
@jrose-signal jrose-signal changed the title Docs say Sleep doesn't implement Unpin, but it does Docs prose say Sleep doesn't implement Unpin, but sidebar does Aug 1, 2023
@sfackler
Copy link
Contributor

sfackler commented Aug 1, 2023

The definition of the impl comes from pin project, and is conditional:

impl<'__pin> Unpin for Sleep
where
    __Origin<'__pin>: Unpin,

It ends up meaning that Sleep is not Unpin.

@taiki-e
Copy link
Member

taiki-e commented Aug 1, 2023

This is a rustdoc bug: rust-lang/rust#80481

We may want to mention in the documentation that rustdoc cannot display this correctly.

@taiki-e taiki-e removed the C-bug Category: This is a bug. label Aug 1, 2023
@Darksonn Darksonn added the M-time Module: tokio/time label Aug 1, 2023
@Darksonn
Copy link
Contributor

Darksonn commented Aug 1, 2023

It's a bit annoying, but we could fix this by not using pin-project-lite for Sleep. Or maybe we could get a feature in pin-project-lite that supports this, where we can say "this type should never be Unpin" and have the macro output something that makes the docs say that its never Unpin?

@taiki-e
Copy link
Member

taiki-e commented Aug 1, 2023

I think it is ok to add pin-project's !Unpin equivalent to pin-project-lite and hide the Unpin implementation from the document.

As I said previously, I do not favor making our code vulnerable to human error to avoid rustdoc issue.

@Darksonn
Copy link
Contributor

Darksonn commented Aug 1, 2023

Yeah, I know. I also agree that it would be vastly preferable to make the macro support it directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-time Module: tokio/time
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants