ACP: Support `NonZeroU16` in `ToSocketAddrs`; ` SocketAddr`'s `From` Implementation

[edward-shen]

Proposal

Thank you for your time and consideration!

Problem statement

core::net::SocketAddr's blanket implementation only accepts u16 as a port type, not NonZeroU16.

Motivating examples or use cases

For services, a non-zero port is often provided as an CLI argument to specify a port to consistently listen to. Common CLI parsers, like clap, support accepting NonZeroU16s as a strongly typed argument. I would like to provide these NonZeroU16s directly to TcpListener::bind which accepts any ToSocketAddrs, and for consistency, SocketAddr to have a From<(I, NonZeroU16>.

Here is minimal example of the desired state:

use std::net::IpAddr;
use std::net::Ipv4Addr;
use std::net::TcpListener;
use std::num::NonZeroU16;

use clap::Parser;

#[derive(clap::Parser)]
struct Args {
    #[clap(short, long, default_value_t = IpAddr::V4(Ipv4Addr::new(0,0,0,0)))]
    address: IpAddr,
    #[clap(short, long, default_value_t = const { NonZeroU16::new(9001).unwrap() })]
    port: NonZeroU16,
}

fn main() {
    let args = Args::parse();

    // Desired
    let _listener = TcpListener::bind((args.address, args.port)).unwrap();
    // Current
    let _listener = TcpListener::bind((args.address, args.port.get())).unwrap();
}

Solution sketch

The solution I would see would be to add a NonZeroU16 variant for all tuple implementations that accept a u16. As an example:

impl ToSocketAddrs for (&str, NonZeroU16) {
    type Iter = <(&str, u16) as ToSocketAddrs>::Iter;

    fn to_socket_addrs(&self) -> io::Result<Self::Iter> {
        (self.0, self.1.get()).to_socket_addrs()
    }
}

Similar impls would be created for the following tuples:

• (IpAddr, u16)
• (String u16)
• (Ipv4Addr, u16)
• (Ipv6Addr, u16)

Additionally, SocketAddr for consistency should similarly have a NonZeroU16 trait implemented:

impl<I: [const] Into<IpAddr>> const From<(I, NonZeroU16)> for SocketAddr {
    fn from(pieces: (I, NonZeroU16)) -> SocketAddr {
        SocketAddr::new(pieces.0.into(), pieces.1.get())
    }
}

Alternatives

As currently presented, this is a minor ergonomic feature with limited scope. The alternative is to call .get() on the NonZeroU16, and no changes would be needed. However, I think this makes for more consistency library experience.

I considered suggesting a blanket implementation where we accept Into<u16>, but I was concerned that it would be too widely applicable so I did not propose that.

I think an interesting point is that the port for SocketAddrs can be 0: the semantics (for Linux at least) is to have the system pick a random port. I think supporting NonZeroU16 still makes sense despite this because it is a strict subset of u16.

This does ask the question of "Should there be a SocketAddr::new_nonzero constructor?" I don't think the library has existing precedence on adding NonZero constructors, so I did not include that as part of the proposal.

Links and related work

(Left empty)

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

• We think this problem seems worth solving, and the standard library might be the right place to solve it.
• We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

• We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
• We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

[joshtriplett]

We talked about this in today's @rust-lang/libs-api meeting. We had mixed feelings on this. Some were weakly supportive, some were weakly opposed. Where we settled was this: could you please do a crater check run, testing if this breaks inference for any crate in the ecosystem? If it doesn't, we'd consider adding this. If it does, it's not worth it.

[edward-shen]

@joshtriplett: thanks for the update!

I tried summoning bors try but it looks like I'm lacking permissions! I think I'll need some help in rust-lang/rust#155983 summoning the bots to do a crater run.

[programmerjake]

one other issue this might cause: if/when integer literals start being able to have type NonZero<Int> (e.g. rust-lang/rfcs#3786), then that would make the following ambiguous since the 22 could be either u16 or NonZero<u16>:

fn connect_to_my_ssh() -> io::Result<TcpStream> {
    TcpStream::connect(("127.0.0.1", 22))
}

[kennytm]

inference breakage caused by rust-lang/rfcs#3786 it should be a problem of NonZero literal, not this ACP.

if TcpStream::connect(("127.0.0.1", 22)) can become ambiguous because both (&str, u16) and (&str, NonZero<u16>) implements ToSocketAddrs, we could also say that 99_u32 / 22 is also ambiguous because u32 implements both Div<u32> and Div<NonZero<u32>>, and that's a much bigger problem to solve for that RFC. and if that RFC can resolve the Div ambiguity i don't see how the ToSocketAddrs ambiguity would then still be a blocker.

[joshtriplett]

@kennytm This could come up with things other than literals, as well. For instance, consider "parse value from string, use value as port number". Right now, the "only one" rules might allow inferring u16.

That said, I stand by the point that if crater doesn't turn up any issues we could add this.

[Amanieu]

We discussed this in the @rust-lang/libs-api meeting. There were stronger feeling this time that we shouldn't add this, in particular because the benefit is so weak (it only saves 1 type conversion from NonZeroU16 to u16). Fundamentally a SockAddr is an address + port pair where the port is a u16, it's the user's responsibility to produce a u16 where this is required. As such we are rejecting this ACP.

[edward-shen]

Thank you all again for your time and consideration for this idea!