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

Un-deprecate IpAddr #1451

Closed
canndrew opened this issue Jan 8, 2016 · 16 comments
Closed

Un-deprecate IpAddr #1451

canndrew opened this issue Jan 8, 2016 · 16 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@canndrew
Copy link
Contributor

canndrew commented Jan 8, 2016

The IpAddr type recently got deprecated from the standard library.

This means libstd now has:

  • A type for (ipv4 address, port) pairs: SocketAddrV4
  • A type for (ipv6 address, port) pairs: SocketAddrV6
  • A type for (ip address, port) pairs: SocketAddr
  • A type for ipv4 addresses: Ipv4Addr
  • A type for ipv6 addresses: Ipv6Addr
  • But not a type for ip addresses: IpAddr

This is silly. IpAddr fits in perfectly with the other types here. It's useful to be able to decompose a SocketAddr into IpAddr and port components and to build one out of these components. It we're going to remove IpAddr then shouldn't SocketAddr be removed aswell? It's very odd to not have a type for generic IP addresses but to have one for generic IP address + port.

There's also at least one place in the standard library itself where IpAddr is needed. The lookup_host function returns an iterator of SocketAddrs with a dummy port because it doesn't have IP address type to use.

The deprecation notice describes IpAddr as being "too small to pull it's weight". Isn't being small a good thing? A type that only takes up a few lines of code doesn't come with much maintainance burden. Surely it's large, complicated types that implement specialised functionality that deserve to be spun off into their own libraries?

Can we just put IpAddr back?

@tshepang
Copy link
Member

tshepang commented Jan 8, 2016

"too small to pull it's weight" looks like it means "not enough functionality to deserve a place in std"

@vinipsmaker
Copy link

"too small to pull it's weight" looks like it means "not enough functionality to deserve a place in std"

This little feature saves a lot of typing. I'm just having to repeat myself every time I need this little feature.

@canndrew
Copy link
Contributor Author

canndrew commented Jan 8, 2016

Just being the Item of LookupHost should be enough functionality.

@tshepang
Copy link
Member

tshepang commented Jan 8, 2016

LookupHost is not even stable

@sfackler
Copy link
Member

sfackler commented Jan 8, 2016

We want to move DNS lookup out of tree and deprecate the in-tree functionality so the API can be improved: rust-lang/rust#27705.

@arcnmx
Copy link

arcnmx commented Jan 10, 2016

+1 this is a very useful type to have a common definition of, and fits in with the rest of std's net types. The ip crate works but really shouldn't be necessary, even if the type isn't used much as an input by std::net APIs themselves.

@dimbleby
Copy link

+1

It certainly makes no sense to have SocketAddr but not IpAddr. That's not intended to encourage deprecation of SocketAddr: I'd much rather have IpAddr back.

@dirvine
Copy link

dirvine commented Jan 13, 2016

+1 for IpAddr being returned. The workarounds are possible with ip crate or own copy etc. but ends up with very messy files and code that is more confusing than it needs to be. This seems to be well used by many projects and seems everyone is working around something in almost the same way. So makes sense to have this in std, esp if socketaddr is, which is really helpful.

@SimonSapin
Copy link
Contributor

@sfackler “DNS lookup” includes the de-facto-stable implementations of ToSocketAddrs for (&'a str, u16) and str. (Only they attempt to parse an IP address before asking libc/the system to resolve a name.) Are they moving out of tree too?

@alexcrichton I remember discussing with you criteria for what should be in std or not. Is that written down somewhere? One of them is when the ecosystem benefits from everyone using the same “standardized” types or traits, allowing interoperability between libraries that don’t depend on each other. I std::net exists at all, I think IpAddr qualifies.

+1

@ustulation
Copy link

+1

@sfackler
Copy link
Member

@SimonSapin Those trait implementations can't move, so no, they're staying where they are.

@wenLiangcan
Copy link

+1

@dimbleby
Copy link

I had an issue report the other day pointing out another reason that the ip crate is not a fully satisfactory replacement: it's not possible for a crate to implement ToSocketAddrs on the (ip::IpAddr, u16) pair.

Not the worst thing in the world, for sure, but for whatever it is worth this speaks in favour of reinstating std::net::IpAddr.

@jfager
Copy link

jfager commented Jan 25, 2016

Haven't we already gone through this dance? #988 It's a tiny unifying enum for a bog-standard use case that multiple people on multiple occasions have said they use and will need to reimplement if it disappears. Please just keep it.

@alexcrichton
Copy link
Member

The libs team discussed this at triage today, and the conclusion was that given how much support there is for this type that we can likely just change the deprecated tags to stable. The type has already gone through an FCP and is a pretty minor surface area, so there's not necessarily any need for another FCP.

@aturon
Copy link
Member

aturon commented Feb 6, 2016

Yep, it was clearly the wrong decision to deprecate -- there's a clear community consensus to ship this, and the rationale for deprecation essentially boiled down to a concern about minimalism in std; there's no real maintenance burden here.

In any case, I've opened a PR to revert, which actually stabilizes the enum as of 1.7 (currently in beta). That will require a backport.

aturon added a commit to aturon/rust that referenced this issue Feb 9, 2016
After [considerable
pushback](rust-lang/rfcs#1451), it's clear
that there is a community consensus around providing `IpAddr` in the
standard library, together with other APIs using it.

This commit reverts from deprecated status directly to stable. The
deprecation landed in 1.6, which has already been released, so the
stabilization is marked for 1.7 (currently in beta; will require a backport).
bors added a commit to rust-lang/rust that referenced this issue Feb 10, 2016
After [considerable pushback](rust-lang/rfcs#1451), it's clear that there is a community consensus around providing `IpAddr` in the standard library, together with other APIs using it.

This commit reverts from deprecated status directly to stable. The deprecation landed in 1.6, which has already been released, so the stabilization is marked for 1.7 (currently in beta; will require a backport).

r? @alexcrichton
alexcrichton pushed a commit to alexcrichton/rust that referenced this issue Feb 11, 2016
After [considerable
pushback](rust-lang/rfcs#1451), it's clear
that there is a community consensus around providing `IpAddr` in the
standard library, together with other APIs using it.

This commit reverts from deprecated status directly to stable. The
deprecation landed in 1.6, which has already been released, so the
stabilization is marked for 1.7 (currently in beta; will require a backport).
@Centril Centril added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests