Support for peer_addr for UdpSocket

[MOZGIII]

I'm trying to test-use the unstable rust feature from here: rust-lang/rust#59127
It would be great if this crate provided the peer_addr the same way the std socket does.

It's ok if we hide it with the feature flag.

[Thomasdezeeuw]

If it moves to stable I think we can do this for 0.7.

[Thomasdezeeuw]

@MOZGIII You want to create a pr for this?

[MOZGIII]

Sure. I'd rather enable support for this even though the std part is in nightly.
It should be possible to hide the method under a feature flag - this way it'll be opt-in.

I'll also look into if it's possible to enable it automatically if the nightly #![feature(udp_peer_addr)] is enabled somewhere else in the code, but I guess it's not possible currently.

Any advice?

[Thomasdezeeuw]

You could add an empty nightly feature to Cargo.toml (for example see https://github.com/Thomasdezeeuw/gaea/blob/76b8f797b9416efdac396f9d9b99501ed143cdaf/Cargo.toml#L36-L37).

Next enable the feature when nightly is available: #![cfg_attr(feature = "nightly", feature(udp_peer_addr))] in src/lib.rs.

And finally add #[cfg(feature = "nightly")] before the method (see https://github.com/Thomasdezeeuw/gaea/blob/76b8f797b9416efdac396f9d9b99501ed143cdaf/src/sys/unix/pipe.rs#L137-L140).

I think that should work.

[carllerche]

The master branch is targeting nightly atm. 0.7 will be released when At least the iovec types are stable.

[Thomasdezeeuw]

IoSlice(previously IoVec) will be stablised in 1.36.0, I've just asked if peer_addr can be stablised for 1.37.0 (rust-lang/rust#59127 (comment)).

[carllerche]

My personal preference is to avoid adding feature flags. If you need access to that function, you can always impl it yourself w/ an ext trait or a custom type.

[MOZGIII]

@carllerche How would I do that in this case? I'd be ok with actually having a custom way of calling this function that's implemented in my project - I just don't want to go through as_raw_fd and co cause that just sounds way too complicated for what I need and I already have a simpler workaround for now. Or do you mean in this crate? If we're talking about crate - adding an ext trait would change the public API in a way I like less than adding a feature flag here - I feel like feature flags fit naturally here.

@Thomasdezeeuw I'm afraid that #![cfg_attr(feature = "nightly", feature(udp_peer_addr))] won't be picked up by mio if I enable udp_peer_addr for my crate - cause they're different compilation units. It might be that udp_peer_addr can be enabled per compilation unit, and not for the whole build (at least that would make sense to me - but I need to actually get to it and test).

Btw, technically, my code might not even need to be aware of std's udp_peer_addr if I only use the mio's wrapper! Still would have to use nightly compiler for the whole build though.

[MOZGIII]

@Thomasdezeeuw I misunderstood what #![cfg_attr(feature = "nightly", feature(udp_peer_addr))] does.

I'd rather explicitly add the udp_peer_addr feature: #![cfg_attr(feature = "udp_peer_addr", feature(udp_peer_addr))]. It is concrete and can easily be deprecated after the udp_peer_addr stabilizes. Any downsides to that?

[carllerche]

We can only add nightly features on the master branch that are stabilized w/ a target target date for landing on stable. Anything else adds too much of a maintenance burden.

[MOZGIII]

Ok, makes sense. Maybe then it makes more sense to just wait a bit on the PR right now.

[MOZGIII]

Hey! Just an update: udp_peer_addr is being stabilized right now, and it's in the merge process. Let's resume our discussion on this issue!

[Thomasdezeeuw]

I saw, I'm ok with targeting 1.39 (in which udp_peer_addr will likely be stable). @carllerche you ok with that?

[carllerche]

1.39 sounds good to me!

[Thomasdezeeuw]

udp_peer_addr was just stabilised in 1.40 (rust-lang/rust#64728, rust-lang/rust#59127). I'm ok with targeting 1.40 as async also didn't make it into 1.39.

[carllerche]

I'm pretty sure async / await is landing as part of 1.39.

[MOZGIII]

Fun fact: at my project we changed the networking logic and we don't need this API anymore. Not that it should't be there, but the adoption sure is slow. 😄

[Thomasdezeeuw]

Blocked on #1381, it's much easier to add once we use socket2.

[Thomasdezeeuw]

Fixed by #1436.