Skip to content

Commit

Permalink
Rollup merge of #107429 - tgross35:from-bytes-until-null-stabilizatio…
Browse files Browse the repository at this point in the history
…n, r=dtolnay

Stabilize feature `cstr_from_bytes_until_nul`

This PR seeks to stabilize `cstr_from_bytes_until_nul`.

Partially addresses #95027

This function has only been on nightly for about 10 months, but I think it is simple enough that there isn't harm discussing stabilization. It has also had at least a handful of mentions on both the user forum and the discord, so it seems like it's already in use or at least known.

This needs FCP still.

Comment on potential discussion points:
- eventual conversion of `CStr` to be a single thin pointer: this function will still be useful to provide a safe way to create a `CStr` after this change.
- should this return a length too, to address concerns about the `CStr` change? I don't see it as being particularly useful, and it seems less ergonomic (i.e. returning `Result<(&CStr, usize), FromBytesUntilNulError>`). I think users that also need this length without the additional `strlen` call are likely better off using a combination of other methods, but this is up for discussion
- `CString::from_vec_until_nul`: this is also useful, but it doesn't even have a nightly implementation merged yet. I propose feature gating that separately, as opposed to blocking this `CStr` implementation on that

Possible alternatives:

A user can use `from_bytes_with_nul` on a slice up to `my_slice[..my_slice.iter().find(|c| c == 0).unwrap()]`. However; that is significantly less ergonomic, and is a bit more work for the compiler to optimize compared the direct `memchr` call that this wraps.

## New stable API

```rs
// both in core::ffi

pub struct FromBytesUntilNulError(());

impl CStr {
    pub const fn from_bytes_until_nul(
        bytes: &[u8]
    ) -> Result<&CStr, FromBytesUntilNulError>
}
```

cc ```@ericseppanen``` original author, ```@Mark-Simulacrum``` original reviewer, ```@m-ou-se``` brought up some issues on the thin pointer CStr

```@rustbot``` modify labels: +T-libs-api +needs-fcp
  • Loading branch information
compiler-errors authored Feb 9, 2023
2 parents a478b83 + 877e9f5 commit aee4570
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 13 deletions.
1 change: 0 additions & 1 deletion library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@
#![feature(const_eval_select)]
#![feature(const_pin)]
#![feature(const_waker)]
#![feature(cstr_from_bytes_until_nul)]
#![feature(dispatch_from_dyn)]
#![feature(error_generic_member_access)]
#![feature(error_in_core)]
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ impl Error for crate::ffi::FromBytesWithNulError {
}
}

#[unstable(feature = "cstr_from_bytes_until_nul", issue = "95027")]
#[stable(feature = "cstr_from_bytes_until_nul", since = "CURRENT_RUSTC_VERSION")]
impl Error for crate::ffi::FromBytesUntilNulError {}

#[unstable(feature = "get_many_mut", issue = "104642")]
Expand Down
11 changes: 5 additions & 6 deletions library/core/src/ffi/c_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,10 @@ impl FromBytesWithNulError {
/// This error is created by the [`CStr::from_bytes_until_nul`] method.
///
#[derive(Clone, PartialEq, Eq, Debug)]
#[unstable(feature = "cstr_from_bytes_until_nul", issue = "95027")]
#[stable(feature = "cstr_from_bytes_until_nul", since = "CURRENT_RUSTC_VERSION")]
pub struct FromBytesUntilNulError(());

#[unstable(feature = "cstr_from_bytes_until_nul", issue = "95027")]
#[stable(feature = "cstr_from_bytes_until_nul", since = "CURRENT_RUSTC_VERSION")]
impl fmt::Display for FromBytesUntilNulError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "data provided does not contain a nul")
Expand Down Expand Up @@ -306,8 +306,6 @@ impl CStr {
///
/// # Examples
/// ```
/// #![feature(cstr_from_bytes_until_nul)]
///
/// use std::ffi::CStr;
///
/// let mut buffer = [0u8; 16];
Expand All @@ -322,8 +320,9 @@ impl CStr {
/// assert_eq!(c_str.to_str().unwrap(), "AAAAAAAA");
/// ```
///
#[unstable(feature = "cstr_from_bytes_until_nul", issue = "95027")]
#[rustc_const_unstable(feature = "cstr_from_bytes_until_nul", issue = "95027")]
#[rustc_allow_const_fn_unstable(const_slice_index)]
#[stable(feature = "cstr_from_bytes_until_nul", since = "CURRENT_RUSTC_VERSION")]
#[rustc_const_stable(feature = "cstr_from_bytes_until_nul", since = "CURRENT_RUSTC_VERSION")]
pub const fn from_bytes_until_nul(bytes: &[u8]) -> Result<&CStr, FromBytesUntilNulError> {
let nul_pos = memchr::memchr(0, bytes);
match nul_pos {
Expand Down
15 changes: 12 additions & 3 deletions library/core/src/slice/memchr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,29 @@ const USIZE_BYTES: usize = mem::size_of::<usize>();
/// bytes where the borrow propagated all the way to the most significant
/// bit."
#[inline]
#[rustc_const_stable(feature = "const_memchr", since = "1.65.0")]
const fn contains_zero_byte(x: usize) -> bool {
x.wrapping_sub(LO_USIZE) & !x & HI_USIZE != 0
}

#[cfg(target_pointer_width = "16")]
#[inline]
#[cfg(target_pointer_width = "16")]
#[rustc_const_stable(feature = "const_memchr", since = "1.65.0")]
const fn repeat_byte(b: u8) -> usize {
(b as usize) << 8 | b as usize
}

#[cfg(not(target_pointer_width = "16"))]
#[inline]
#[cfg(not(target_pointer_width = "16"))]
#[rustc_const_stable(feature = "const_memchr", since = "1.65.0")]
const fn repeat_byte(b: u8) -> usize {
(b as usize) * (usize::MAX / 255)
}

/// Returns the first index matching the byte `x` in `text`.
#[must_use]
#[inline]
#[must_use]
#[rustc_const_stable(feature = "const_memchr", since = "1.65.0")]
pub const fn memchr(x: u8, text: &[u8]) -> Option<usize> {
// Fast path for small slices.
if text.len() < 2 * USIZE_BYTES {
Expand All @@ -45,6 +49,7 @@ pub const fn memchr(x: u8, text: &[u8]) -> Option<usize> {
}

#[inline]
#[rustc_const_stable(feature = "const_memchr", since = "1.65.0")]
const fn memchr_naive(x: u8, text: &[u8]) -> Option<usize> {
let mut i = 0;

Expand All @@ -60,6 +65,10 @@ const fn memchr_naive(x: u8, text: &[u8]) -> Option<usize> {
None
}

#[rustc_allow_const_fn_unstable(const_cmp)]
#[rustc_allow_const_fn_unstable(const_slice_index)]
#[rustc_allow_const_fn_unstable(const_align_offset)]
#[rustc_const_stable(feature = "const_memchr", since = "1.65.0")]
const fn memchr_aligned(x: u8, text: &[u8]) -> Option<usize> {
// Scan for a single byte value by reading two `usize` words at a time.
//
Expand Down
1 change: 0 additions & 1 deletion library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,6 @@
#![feature(char_error_internals)]
#![feature(char_internals)]
#![feature(core_intrinsics)]
#![feature(cstr_from_bytes_until_nul)]
#![feature(cstr_internals)]
#![feature(duration_constants)]
#![feature(error_generic_member_access)]
Expand Down
1 change: 0 additions & 1 deletion src/tools/miri/tests/pass-dep/shims/pthreads.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//@ignore-target-windows: No libc on Windows
#![feature(cstr_from_bytes_until_nul)]
use std::ffi::{CStr, CString};
use std::thread;

Expand Down

0 comments on commit aee4570

Please sign in to comment.