Skip to content

Commit

Permalink
implement remove() API
Browse files Browse the repository at this point in the history
* add unit tests
* add doc tests
* add prop tests
* add to fuzzing harness
  • Loading branch information
ParkMyCar committed Jul 25, 2022
1 parent 21eee02 commit 471b620
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 0 deletions.
61 changes: 61 additions & 0 deletions compact_str/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,67 @@ impl CompactString {
self.repr.push_str(s)
}

/// Removes a [`char`] from this [`CompactString`] at a byte position and returns it.
///
/// This is an *O*(*n*) operation, as it requires copying every element in the
/// buffer.
///
/// # Panics
///
/// Panics if `idx` is larger than or equal to the [`CompactString`]'s length,
/// or if it does not lie on a [`char`] boundary.
///
/// # Examples
///
/// ### Basic usage:
///
/// ```
/// # use compact_str::CompactString;
/// let mut c = CompactString::from("hello world");
///
/// assert_eq!(c.remove(0), 'h');
/// assert_eq!(c, "ello world");
///
/// assert_eq!(c.remove(5), 'w');
/// assert_eq!(c, "ello orld");
/// ```
///
/// ### Past total length:
///
/// ```should_panic
/// # use compact_str::CompactString;
/// let mut c = CompactString::from("hello there!");
/// c.remove(100);
/// ```
///
/// ### Not on char boundary:
///
/// ```should_panic
/// # use compact_str::CompactString;
/// let mut c = CompactString::from("🦄");
/// c.remove(1);
/// ```
#[inline]
pub fn remove(&mut self, idx: usize) -> char {
// get the char
let ch = match self[idx..].chars().next() {
Some(ch) => ch,
None => panic!("cannot remove a char from the end of a string"),
};

let after_char = idx + ch.len_utf8();
let len = self.len();
unsafe {
core::ptr::copy(
self.as_ptr().add(after_char),
self.as_mut_ptr().add(idx),
len - after_char,
);
self.set_len(len - (after_char - idx));
}
ch
}

/// Forces the length of the [`CompactString`] to `new_len`.
///
/// This is a low-level operation that maintains none of the normal invariants for
Expand Down
55 changes: 55 additions & 0 deletions compact_str/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::borrow::Cow;
use std::num;
use std::str::FromStr;

use proptest::collection::SizeRange;
use proptest::prelude::*;
use proptest::strategy::Strategy;
use test_strategy::proptest;
Expand Down Expand Up @@ -40,6 +41,11 @@ pub fn rand_unicode_with_max_len(len: usize) -> impl Strategy<Value = String> {
})
}

/// [`proptest::strategy::Strategy`] that generates [`String`]s with up to `len` bytes
pub fn rand_unicode_with_range(range: impl Into<SizeRange>) -> impl Strategy<Value = String> {
proptest::collection::vec(proptest::char::any(), range).prop_map(|v| v.into_iter().collect())
}

/// generates groups upto 40 strings long of random unicode strings, upto 80 chars long
fn rand_unicode_collection() -> impl Strategy<Value = Vec<String>> {
proptest::collection::vec(rand_unicode(), 0..40)
Expand Down Expand Up @@ -219,6 +225,36 @@ fn proptest_truncate(#[strategy(rand_unicode())] mut control: String, val: u8) {
}
}

#[proptest]
#[cfg_attr(miri, ignore)]
fn proptest_remove(#[strategy(rand_unicode_with_range(1..80))] mut control: String, val: u8) {
let initial_len = control.len();
let mut compact = CompactString::new(&control);

let idx = control
.char_indices()
.into_iter()
.cycle()
.nth(val as usize)
.unwrap_or_default()
.0;

let control_char = control.remove(idx);
let compact_char = compact.remove(idx);

prop_assert_eq!(control_char, compact_char);
prop_assert_eq!(control_char, compact_char);
prop_assert_eq!(control.len(), compact.len());

// If we started as heap allocated, we should stay heap allocated. This prevents us from
// needing to deallocate the buffer on the heap
if initial_len > MAX_SIZE {
prop_assert!(compact.is_heap_allocated());
} else {
prop_assert!(!compact.is_heap_allocated());
}
}

#[test]
fn test_const_creation() {
const EMPTY: CompactString = CompactString::new_inline("");
Expand Down Expand Up @@ -1025,6 +1061,25 @@ fn test_insert() {
);
}

#[test]
fn test_remove() {
let mut control = String::from("🦄🦀hello🎶world🇺🇸");
let mut compact = CompactString::from(&control);

assert_eq!(control.remove(0), compact.remove(0));
assert_eq!(control, compact);
assert_eq!(compact, "🦀hello🎶world🇺🇸");

let music_idx = control
.char_indices()
.find(|(_idx, c)| *c == '🎶')
.map(|(idx, _c)| idx)
.unwrap();
assert_eq!(control.remove(music_idx), compact.remove(music_idx));
assert_eq!(control, compact);
assert_eq!(compact, "🦀helloworld🇺🇸");
}

#[test]
fn test_with_capacity_16711422() {
// Fuzzing with AFL on a 32-bit ARM arch found this bug!
Expand Down
15 changes: 15 additions & 0 deletions fuzz/src/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ pub enum Action<'a> {
SplitOff(u8),
/// Extract a range
Drain(u8, u8),
/// Remove a `char`
Remove(u8),
}

impl Action<'_> {
Expand Down Expand Up @@ -204,6 +206,19 @@ impl Action<'_> {
assert_eq!(control.as_str(), compact.as_str());
assert_eq!(compact.capacity(), compact_capacity);
}
Remove(val) => {
let idx = to_index(control, val);

// if the strings are empty, we can't remove anything
if control.is_empty() && compact.is_empty() {
assert_eq!(idx, 0);
return;
}

assert_eq!(control.remove(idx), compact.remove(idx));
assert_eq!(control, compact);
assert_eq!(control.len(), compact.len());
}
}
}
}
Expand Down

0 comments on commit 471b620

Please sign in to comment.