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

Consider renaming Async::new() #36

Closed
ghost opened this issue Apr 27, 2020 · 9 comments
Closed

Consider renaming Async::new() #36

ghost opened this issue Apr 27, 2020 · 9 comments
Labels
request for comments Feedback, thoughts, suggestions, opinions
Milestone

Comments

@ghost
Copy link

ghost commented Apr 27, 2020

Since Async::new() should really only be used for networking and not for stdin/stderr/stdout/files, what if we renamed Async::new() to Async::socket() to prevent people from putting files etc. inside Async and expecting things to work?

That said, there are some things that aren't sockets and can be used with Async like timerfd and notify, but we can say those are socket-like things.

On Windows, Async::new() only accepts T: AsRawSocket so you really can only use sockets with it - the situation there is perfect. On Unix, there's just AsRawFd and there's not really any way to distinguish between sockets and other kinds of file descriptors.

@shekohex
Copy link

shekohex commented Apr 27, 2020

What about removing the Async::new() entirely and use Async::try_from() for all T: AsRawFd?
Maybe I should open a PR with that? (I mean for the impl TryFrom<T: AsRawFd> for Async<T>)

@sdroege
Copy link

sdroege commented Apr 27, 2020

Maybe instead of "socket", "pollable fd" would be a better term? It seems weird to call the various other kinds of pollable fds on Linux "sockets" :)

On Windows, Async::new() only accepts T: AsRawSocket so you really can only use sockets with it - the situation there is perfect.

This also seems like a good reason to rename, as one might want to add an Async::handle() in the future that works on HANDLEs, for example.

@ghost
Copy link
Author

ghost commented Apr 27, 2020

@shekohex Interesting, haven't thought of that! But I worry people would do Async::try_from(file)?

@sdroege I like that - so we can explain Async is only for pollable things, which files and stdio aren't.

@sdroege
Copy link

sdroege commented Apr 27, 2020

@sdroege I like that - so we can explain Async is only for pollable things, which files and stdio aren't.

stdio/out/err are actually pollable or not (just not on Windows)? I remember doing that quite a few times.

@pkorotkov
Copy link

pkorotkov commented Apr 27, 2020

@stjepang As long as the AsRawFd bound is kept, you just can't stop people writing Async::try_from(file)?. However, you shouldn't because the method's name already suggests a known idiomatic approach most rust developers are familiar with.

@ghost
Copy link
Author

ghost commented Apr 29, 2020

How about Async::pollable()?

@ghost ghost mentioned this issue May 3, 2020
@ghost ghost added the request for comments Feedback, thoughts, suggestions, opinions label May 3, 2020
@bugaevc
Copy link

bugaevc commented May 8, 2020

stdio/out/err are actually pollable or not (just not on Windows)? I remember doing that quite a few times.

That of course depends on what your stdio streams happen to be. If they're on-disk files, they're not pollable. If they're ptys, or sockets, or pipes, they are 🙂 So it's definitely true that you cannot assume they're always pollable.

@bugaevc
Copy link

bugaevc commented May 8, 2020

How about Async::pollable()?

Perhaps Async::polling(smth)? Though that sounds a bit Swift-ish.

@ghost ghost added this to the async-io milestone Jun 22, 2020
@ghost
Copy link
Author

ghost commented Aug 14, 2020

I decided to not change this :) It seems that the original issue hasn't come up again since the initial release of smol. That may be partly because more people have become familiarized with Async vs Unblock distinction, and partly because there are now higher-level crates like async-net and async-fs.

@ghost ghost closed this as completed Aug 14, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request for comments Feedback, thoughts, suggestions, opinions
Development

No branches or pull requests

4 participants