feat: Implement CompactString::remove()

[ParkMyCar]

• add unit tests
• add doc tests
• add prop tests
• add to fuzzing harness

Fixes #141

[Kijewski]

You could use char_indices() instead of chars() to omit the len_utf8() call.

[Kijewski]

You could use let s = self.as_mut_str(); and operate on s throughout the function, so the pointers don't have to be calculated multiple times, and miri understands what you are doing.

[ParkMyCar]

I'm not sure how char_indices() would help here? Wouldn't that give us the index of the char we want to remove, which should be the same as the idx argument?

Is there a method we can use with &mut str to do the copying? Maybe you meant let ptr = self.as_mut_ptr()? This prevents the need to calculate the pointer, and fixes Miri :)

[Kijewski]

I meant something like that:

pub fn remove(&mut self, idx: usize) -> char {
    let s = self.as_mut_str();
    let old_len = s.len();

    let substr = &mut s[idx..];
    let (ch_len, ch) = substr.char_indices()
        .next()
        .expect("cannot remove a char from the end of a string");

    // shift everything back one character
    unsafe {
        core::ptr::copy(
            substr.as_ptr().add(ch_len),
            substr.as_mut_ptr(),
            old_len - (idx + ch_len),
        );
        self.set_len(old_len - ch_len);
    }
    ch
}

[ParkMyCar]

Ahh I see, thanks for providing the example :)

Here ch_len isn't the length of the char returned, I believe it would actually always be 0 because it's the first character of substr.

We run Miri with -Zmiri-strict-provenance and getting two pointers at once seems to violate the stacked borrow rules. Although I like your idea of operating on substr, I refactored the impl to work similar to what you've described.

[Kijewski]

Oops, you're absolutely right. I confused myself how char_indices() works. :)

[Kijewski]

to_index() can also return s.len(). This should have caused some crashes in the fuzzer.

[ParkMyCar]

Good call out! It looks like as it exists today to_index() calls .cycle() before it .chain([s.len()]), which was causing s.len() to never get emitted, since .cycle() never ends. I updated to_index() to fix this and added tests to make sure it's doing what we want