Skip to content

Commit

Permalink
Rollup merge of #128137 - GrigorenkoPV:cstr-derive, r=dtolnay
Browse files Browse the repository at this point in the history
CStr: derive PartialEq, Eq; add test for Ord

While working on #128046, I've spotted a peculiarity: `CStr` has `PartialEq, Eq, PartialOrd, Ord` implemented manually and not derived.

While we can't derive `PartialOrd, Ord` (due to inner `[c_char]` being `[i8]` or `[u8]` on different platforms), we *can* derive `PartialEq, Eq` (I think), allowing as to remove `#[allow(clippy::derived_hash_with_manual_eq)]` as well.

(I really hope `c_char: Eq` on all platforms)
  • Loading branch information
matthiaskrgr authored Jul 25, 2024
2 parents c96311b + cf9816c commit 512277f
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 11 deletions.
15 changes: 4 additions & 11 deletions library/core/src/ffi/c_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ use crate::str;
/// ```
///
/// [str]: prim@str "str"
#[derive(Hash)]
#[derive(PartialEq, Eq, Hash)]
#[stable(feature = "core_c_str", since = "1.64.0")]
#[rustc_has_incoherent_inherent_impls]
#[lang = "CStr"]
Expand All @@ -104,7 +104,6 @@ use crate::str;
// want `repr(transparent)` but we don't want it to show up in rustdoc, so we hide it under
// `cfg(doc)`. This is an ad-hoc implementation of attribute privacy.
#[repr(transparent)]
#[allow(clippy::derived_hash_with_manual_eq)]
pub struct CStr {
// FIXME: this should not be represented with a DST slice but rather with
// just a raw `c_char` along with some form of marker to make
Expand Down Expand Up @@ -678,15 +677,9 @@ impl CStr {
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl PartialEq for CStr {
#[inline]
fn eq(&self, other: &CStr) -> bool {
self.to_bytes().eq(other.to_bytes())
}
}
#[stable(feature = "rust1", since = "1.0.0")]
impl Eq for CStr {}
// `.to_bytes()` representations are compared instead of the inner `[c_char]`s,
// because `c_char` is `i8` (not `u8`) on some platforms.
// That is why this is implemented manually and not derived.
#[stable(feature = "rust1", since = "1.0.0")]
impl PartialOrd for CStr {
#[inline]
Expand Down
1 change: 1 addition & 0 deletions library/core/tests/ffi.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
mod cstr;
15 changes: 15 additions & 0 deletions library/core/tests/ffi/cstr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
use core::ffi::CStr;

#[test]
fn compares_as_u8s() {
let a: &CStr = c"Hello!"; // Starts with ascii
let a_bytes: &[u8] = a.to_bytes();
assert!((..0b1000_0000).contains(&a_bytes[0]));

let b: &CStr = c"こんにちは!"; // Starts with non ascii
let b_bytes: &[u8] = b.to_bytes();
assert!((0b1000_0000..).contains(&b_bytes[0]));

assert_eq!(Ord::cmp(a, b), Ord::cmp(a_bytes, b_bytes));
assert_eq!(PartialOrd::partial_cmp(a, b), PartialOrd::partial_cmp(a_bytes, b_bytes));
}
1 change: 1 addition & 0 deletions library/core/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ mod clone;
mod cmp;
mod const_ptr;
mod convert;
mod ffi;
mod fmt;
mod future;
mod hash;
Expand Down

0 comments on commit 512277f

Please sign in to comment.