Skip to content

Commit

Permalink
Rollup merge of #116714 - WaffleLapkin:order-the-order, r=joshtriplett
Browse files Browse the repository at this point in the history
Derive `Ord`, `PartialOrd` and `Hash` for `SocketAddr*`

Fixes #116711

The main pain of this PR is to fix the buggy impl of `Ord` for `SocketAddrV6`, which ignored half of the fields (while `PartialEq` is derived):
https://github.com/rust-lang/rust/blob/4603f0b8afb495ae56cd4c8f70d5d478d906ac54/library/core/src/net/socket_addr.rs#L99-L106

https://github.com/rust-lang/rust/blob/4603f0b8afb495ae56cd4c8f70d5d478d906ac54/library/core/src/net/socket_addr.rs#L676

For me it looks like a simple copy-paste error made in #72239 (copy from v4 impl) (cc `@hch12907),` as I don't see this behavior being mentioned anywhere on the PR and it also does not respect `cmp` trait "rules". I also do not see any reasons for those impls to _not_ be derived.

It's a shame we did not notice this for 28 versions/3 years. I guess this is a bug fix, but I'm not sure what the process here should be.

r? libs
  • Loading branch information
matthiaskrgr authored Oct 24, 2023
2 parents f3e18e4 + 5c13c69 commit 845c414
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 49 deletions.
51 changes: 2 additions & 49 deletions library/core/src/net/socket_addr.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::cmp::Ordering;
use crate::fmt::{self, Write};
use crate::hash;
use crate::net::{IpAddr, Ipv4Addr, Ipv6Addr};

use super::display_buffer::DisplayBuffer;
Expand Down Expand Up @@ -63,7 +61,7 @@ pub enum SocketAddr {
/// assert_eq!(socket.ip(), &Ipv4Addr::new(127, 0, 0, 1));
/// assert_eq!(socket.port(), 8080);
/// ```
#[derive(Copy, Clone, Eq, PartialEq)]
#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct SocketAddrV4 {
ip: Ipv4Addr,
Expand Down Expand Up @@ -96,7 +94,7 @@ pub struct SocketAddrV4 {
/// assert_eq!(socket.ip(), &Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1));
/// assert_eq!(socket.port(), 8080);
/// ```
#[derive(Copy, Clone, Eq, PartialEq)]
#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct SocketAddrV6 {
ip: Ipv6Addr,
Expand Down Expand Up @@ -644,48 +642,3 @@ impl fmt::Debug for SocketAddrV6 {
fmt::Display::fmt(self, fmt)
}
}

#[stable(feature = "socketaddr_ordering", since = "1.45.0")]
impl PartialOrd for SocketAddrV4 {
#[inline]
fn partial_cmp(&self, other: &SocketAddrV4) -> Option<Ordering> {
Some(self.cmp(other))
}
}

#[stable(feature = "socketaddr_ordering", since = "1.45.0")]
impl PartialOrd for SocketAddrV6 {
#[inline]
fn partial_cmp(&self, other: &SocketAddrV6) -> Option<Ordering> {
Some(self.cmp(other))
}
}

#[stable(feature = "socketaddr_ordering", since = "1.45.0")]
impl Ord for SocketAddrV4 {
#[inline]
fn cmp(&self, other: &SocketAddrV4) -> Ordering {
self.ip().cmp(other.ip()).then(self.port().cmp(&other.port()))
}
}

#[stable(feature = "socketaddr_ordering", since = "1.45.0")]
impl Ord for SocketAddrV6 {
#[inline]
fn cmp(&self, other: &SocketAddrV6) -> Ordering {
self.ip().cmp(other.ip()).then(self.port().cmp(&other.port()))
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl hash::Hash for SocketAddrV4 {
fn hash<H: hash::Hasher>(&self, s: &mut H) {
(self.port, self.ip).hash(s)
}
}
#[stable(feature = "rust1", since = "1.0.0")]
impl hash::Hash for SocketAddrV6 {
fn hash<H: hash::Hasher>(&self, s: &mut H) {
(self.port, &self.ip, self.flowinfo, self.scope_id).hash(s)
}
}
11 changes: 11 additions & 0 deletions library/core/tests/net/socket_addr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ fn compare() {
let v6_1 = "[2001:db8:f00::1002]:23456".parse::<SocketAddrV6>().unwrap();
let v6_2 = "[2001:db8:f00::2001]:12345".parse::<SocketAddrV6>().unwrap();
let v6_3 = "[2001:db8:f00::2001]:23456".parse::<SocketAddrV6>().unwrap();
let v6_4 = "[2001:db8:f00::2001%42]:23456".parse::<SocketAddrV6>().unwrap();
let mut v6_5 = "[2001:db8:f00::2001]:23456".parse::<SocketAddrV6>().unwrap();
v6_5.set_flowinfo(17);

// equality
assert_eq!(v4_1, v4_1);
Expand All @@ -207,6 +210,8 @@ fn compare() {
assert_eq!(SocketAddr::V6(v6_1), SocketAddr::V6(v6_1));
assert!(v4_1 != v4_2);
assert!(v6_1 != v6_2);
assert!(v6_3 != v6_4);
assert!(v6_3 != v6_5);

// compare different addresses
assert!(v4_1 < v4_2);
Expand All @@ -226,6 +231,12 @@ fn compare() {
assert!(v4_3 > v4_1);
assert!(v6_3 > v6_1);

// compare the same address with different scope_id
assert!(v6_3 < v6_4);

// compare the same address with different flowinfo
assert!(v6_3 < v6_5);

// compare with an inferred right-hand side
assert_eq!(v4_1, "224.120.45.1:23456".parse().unwrap());
assert_eq!(v6_1, "[2001:db8:f00::1002]:23456".parse().unwrap());
Expand Down

0 comments on commit 845c414

Please sign in to comment.