Tracking issue for Ipv{4,6}Addr convenience methods

[alexcrichton]

View all comments

The below is a list of methods left to be stabilized under the ip feature. The path forward today is unclear; if you'd like to push through any method on here the libs team is interested in a PR with links to the associated RFCs or other official documentation. Let us know!

• IpAddr::is_documentation
• Ipv6Addr::is_documentation

---

• IpAddr::is_global
• Ipv4Addr::is_global
• Ipv6Addr::is_global

---

• Ipv4Addr::is_shared
• Ipv4Addr::is_ietf_protocol_assignment Remove Ipv4Addr::is_ietf_protocol_assignment #86439
• Ipv4Addr::is_benchmarking
• Ipv4Addr::is_reserved

---

• Ipv6Addr::is_unicast_global
• Ipv6Addr::is_unicast_link_local
• Ipv6Addr::is_unicast_link_local_strict Remove Ipv6Addr::is_unicast_link_local_strict #85819
• Ipv6Addr::is_unicast_site_local Remove Ipv6Addr::is_unicast_site_local #85820
• Ipv6Addr::is_unique_local
• Ipv6Addr::multicast_scope

---

• Ipv6Addr::is_ipv4_mapped
• Ipv6Addr::to_ipv4_mapped Stabilize Ipv6Addr::to_ipv4_mapped #96906
• Ipv6Addr::to_canonical Stabilize {IpAddr, Ipv6Addr}::to_canonical #115955
• IpAddr::to_canonical Stabilize {IpAddr, Ipv6Addr}::to_canonical #115955

Steps

• Implementation std: Add a net module for TCP/UDP #22015
• Implementation Add IpAddr common methods #34694
• Stabilization attempt stabilize the "ip" feature #66584
• Stabilization attempt Stabilize the "IP" feature #76098
• Stabilization PR(s)

Subsets of the listed methods can be stabilized, rather than attempting to stabilize everything at once.

Unresolved Questions

• is_documentation is not up to date Ipv6Addr::is_documentation should be expanded to include RFC 9637's 3ffff::/20 #137821
• Differences between Rust and other languages Stabilize the "IP" feature #76098 (comment)
• More specific case of the above: does the IPv6 unicast interface do what we expect? IPv6 Unicast Interface #85604
• Do the provided methods pass ipcheck? https://github.com/rust-lang/libs-team/tree/93b78eef2e0d455a3e69c05333cd8f276e4e95f1/tools/ipcheck. Last known run: Stabilize the "IP" feature #76098 (comment)

From @KodrAus in #76098 (comment):

• Should we replace the Ipv6Addr::is_unicast_* methods with a Ipv6Addr::unicast_scope method that returns a Ipv6UnicastScope enum (Stabilize the "IP" feature #76098 (comment))?
• Should we change the behavior of Ipv6Addr::to_ipv4 to ignore deprecated IPv4-compatible addresses, or deprecate the whole method in favor of the more correct Ipv6Addr::to_ipv4_mapped method (Stabilize the "IP" feature #76098 (comment))?
• Are we ok with Ipv6Addr::is_* methods now properly considering mapped (non-deprecated) IPv4 addresses? I'd personally be comfortable considering the old behavior a bug.
• Are there any behavioral differences between other language implementations that we should investigate? (Stabilize the "IP" feature #76098 (comment))

[aturon]

I think we should consider this for stabilization in 1.6. Nominating.

[alexcrichton]

🔔 This issue is now entering its cycle-long final comment period for stabilization 🔔

Concretely, we discussed this in the libs meeting and the conclusion was that the boolean accessors are likely ready for stabilization after verifying that they're all the canonical definitions, but the enum-returning variants will likely remain unstable for now.

[briansmith]

What, exactly, is the "ip feature"? Could you link to the RustDoc(s) of the specific things that are supposed to be reviewed?

[SimonSapin]

I think "ip feature" in this context refers to things annotated with #![unstable(feature = "ip", …)], which require #![feature(ip)] to be used.

[briansmith]

I think "ip feature" in this context refers to things annotated with #![unstable(feature = "ip", …)], which require #![feature(ip)] to be used.

I don't mean to be rude, but that's just a restating my question as the answer. if you want people to actually give feedback on the proposal, it should be easier to understand what the proposal is. In this case, it is pretty difficult to tell what is being proposed because the module mixes stable and unstable features.

I noticed that a large part of this module could work in #![no_std] mode. I suggest moving the #![no_std]-compatible parts to core::net, or at least consider how making it work with #![no_std] would affect the API.

It also seems odd that Ipv4Addr and Ipv6Addr have things like is_multicast and is_global but there's no trait that allows code to make these queries generically over those types of addresses. If such a trait were to bad added later, would the existence of these non-trait methods cause problems? If so, it might be worth considering building the trait first.

[steveklabnik]

@briansmith I don't think that's being rude, there's just tension here between "we've been doing this a while so we're a bit short" and "newer people might not know what that is." @SimonSapin leaned a bit towards a literal explanation, but you're right to point out that more detail is good.

I read @alexcrichton 's comment as:

that the boolean accessors are likely ready for stabilization after verifying that they're all the canonical definitions,

http://doc.rust-lang.org/std/net/struct.Ipv4Addr.html and http://doc.rust-lang.org/std/net/struct.Ipv6Addr.html <- all the stuff here that is -> bool

but the enum-returning variants will likely remain unstable for now.

http://doc.rust-lang.org/std/net/struct.Ipv6Addr.html#method.multicast_scope is the only one I see that's unstable.

[huonw]

FWIW, I filed #29221 a while ago, which would make tracking down what these tracking issues refer to slightly easier.

[alexcrichton]

Yes, to be concrete, I was proposing stabilizing:

• Ipv4Addr::is_unspecified
• Ipv4Addr::is_loopback
• Ipv4Addr::is_private
• Ipv4Addr::is_link_local
• Ipv4Addr::is_global
• Ipv4Addr::is_multicast
• Ipv4Addr::is_broadcast
• Ipv4Addr::is_documentation
• Ipv6Addr::is_unspecified
• Ipv6Addr::is_loopback
• Ipv6Addr::is_global
• Ipv6Addr::is_unique_local
• Ipv6Addr::is_unicast_link_local
• Ipv6Addr::is_unicast_site_local
• Ipv6Addr::is_unicast_global
• Ipv6Addr::is_multicast

Note that this is all pending actually verifying that these are standard properties of the respective IP address space and are well known with canonical implementations. I believe they fit this requirement already but would like to double-check.

---

@briansmith

I suggest moving the #![no_std]-compatible parts to core::net

Yeah these things can certainly move around over time (it's backwards compatible to move them at a later date). I'd be a little wary of putting things in core "just because" without a concrete purpose, and these kinda fall into the category I'd be wary of. For example the internal representation of each of these primitives is the libc equivalent (e.g. libc::sockaddr_in6 or libc::in_addr) which unfortunately isn't available in libcore, so if we move it to core we'd have to invent our own storage format.

It also seems odd that Ipv4Addr and Ipv6Addr have things like is_multicast and is_global but there's no trait that allows code to make these queries generically over those types of addresses. If such a trait were to bad added later, would the existence of these non-trait methods cause problems?

Method resolution favors inherent methods (methods defined on the type itself) over trait methods (e.g. impl'd traits plus the trait being in scope), so in that sense we're covered to add a trait at a future date. That being said the standard library doesn't have too many traits like this for abstracting between one or two types, so I would personally want to hold off on this extension for now.

A possible alternative, however, could be adding the common set of methods to IpAddr if we end up stabilizing that as well.

[ollie27]

A couple of issues I have noticed with what we have:

• 0.0.0.0/8, :: and many more ranges shouldn't return true for is_global()
• Ipv6Addr::is_documentation is missing and should be 2001:db8::/32

On a more general note, might some of these functions need to be updated in the future if new ranges are assigned? How would that be handled?