Skip to content

Add Async::{readable_owned, writable_owned} #66

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

Merged
merged 1 commit into from
May 30, 2021
Merged

Conversation

taiki-e
Copy link
Collaborator

@taiki-e taiki-e commented May 23, 2021

This replaces yanked #64.

  • change Readable and Writable futures to contain a reference to Async
  • add Async::{readable_owned, writable_owned} that take self: Arc<Self>

See #64 (comment) (and the comments before and after) for more.

r? @Kestrer @yoshuawuyts @smol-rs/admins

src/reactor.rs Outdated
}
}

struct Ready<S: Borrow<crate::Async<T>>, T> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally I wanted to use AsRef<Source>, as @Kestrer suggested in #64 (comment), but I could not implement it because we had already implemented AsRef<T> for Async<T>.

src/reactor.rs Outdated

/// Future for [`Async::readable_owned`](crate::Async::readable_owned).
#[must_use = "futures do nothing unless you `.await` or poll them"]
pub struct OwnedReadable<T>(Ready<Arc<crate::Async<T>>, T>);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it a bit confusing it's readable_owned but OwnedReadable. Could this be changed to ReadableOwned?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to ReadableOwned/WritableOwned.

@taiki-e taiki-e merged commit 1634117 into master May 30, 2021
@taiki-e taiki-e deleted the taiki-e/owned branch May 30, 2021 06:24
taiki-e referenced this pull request in smol-rs/async-net Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants