move `impl Async<std::net::*>` to `async-net`

[JayceFayne]

I actually thought about proposing this while investigating #18 and now that async-io is at v1.0.x I am not sure if a breaking change directly after v1.0 would be appreciated.

That said I believe it would be a good idea to move all the net related impls meaning impl Async<TcpListener>, impl Async<TcpStream> and so on to async-net. That way we can remove the socket2 dependency from async-io and it would reduce the overall code base. I already looked into this a bit and one obvious downside would be that the current doc tests have to be rewritten since they mostly 12/20 use net related types. Feel free to close this issue if you don't think this is a good idea or if I have simply missed something and it's not possible to implement it that way 😄

Unfortunately, this is not possible to due to coherence rules: Async<T> can only be implemented inside the crate that defined Async. Otherwise, we get a compilation error.

Regarding removing socket2 dependency: I want it removed too, especially considering the fact that socket2 has a lot of unsafe code and async-io is only using a function or two from it. In fact, I want to remove as much unsafe code as possible from the whole dependency tree of smol.

[JayceFayne]

Unfortunately, this is not possible to due to coherence rules: Async<T> can only be implemented inside the crate that defined Async. Otherwise, we get a compilation error.

Ahh I believe you misunderstood me. I know that you cannot impl Async<T> if both Async and T are types that are not defined by the current crate. What I meant was moving the impls to the already existing impls in async-net. E.g. replacing self.0.accept().await? here with the corresponding impl Async<TcpListener>::accept from here. That way there are no impl Async<TcpListener> blocks and the coherence rules don't apply.

Regarding removing socket2 dependency: I want it removed too, especially considering the fact that socket2 has a lot of unsafe code and async-io is only using a function or two from it. In fact, I want to remove as much unsafe code as possible from the whole dependency tree of smol.

Yes I noticed that too! That's why I would really like to move that code from async-io to async-net. To be fair, moving the socket2 dependency from async-io to async-net would not remove unsafe code from smol's dependency tree...:smile:

But that way async-io is really only about async I/O and timers and does not contain any impls for specific async types.

Ah sorry, that clears it up! :) Well, as you noted, async-io is at v1.0 so it'd be a bit unfortunate to bump it to v2.0 now. I'm hoping that replacing socket2 with a few calls to libc would not add much cruft though.

But also, a lot of people like the fact that Async<T> is fully functional so it'd be unfortunate to remove those impls. While it's possible to use Async::<TcpListener>::read_with() in place of Async::<TcpListener>::accept(), it's not possible to do the same with Async::<TcpStream>::connect(). So if we were to remove that method, then it wouldn't be possible to use Async<TcpStream> alone to asynchronously connect to an address.

Gonna close this since it's very unlikely the API changes at this point. Maybe sometime in the future, but not soon.

[JayceFayne]

Gonna close this since it's very unlikely the API changes at this point. Maybe sometime in the future, but not soon.

Yeah, sorry for my late reply. I understand the reasons of keeping the impl Async<std::net::*> blocks in async-io for now.

Ah sorry, that clears it up! :) Well, as you noted, async-io is at v1.0 so it'd be a bit unfortunate to bump it to v2.0 now. I'm hoping that replacing socket2 with a few calls to libc would not add much cruft though.

I'll look into that and provide a pull if I have time in the next weeks 😄

For the longest time I believed this horrible hack was unavoidable: https://github.com/stjepang/async-io/blob/master/src/reactor.rs#L699

But, now I have found a really neat solution to remove it. :) The problem is, removing this hack requires breaking changes because we'd have to remove impls of AsyncRead/AsyncWrite for &Async<T>, as well as async_net::TcpStream and async_net::unix::UnixStream.

So now I'm considering removing this bug once and for all, so this is a great chance to lump in other breaking changes. Which means your proposal is back on the table :)

Actually, never mind - I decided to not do this after all. :)