fitzgen / inlinable_string

An owned, grow-able UTF-8 string that stores small strings inline and avoids heap-allocation.
http://fitzgen.github.io/inlinable_string/inlinable_string/index.html
Other
69 stars 11 forks source link

Support missing std `String` methods #26

Open lo48576 opened 4 years ago

lo48576 commented 4 years ago

String has some new methods not supported by inlinable_string::StringExt trait, and some of them should be implemented using unsafe to perform well (for example, insert_str and retain).

Of course they can be implemented without unsafe (and they would be slow). However, I think inlinable_string crate should implement and test them in some way (possibly using unsafe), before users try to implement by themselves. Use of unsafe should be gathered to upstream crates as possible, rather than written by every downstream user.

Here is a list of missing methods:

They can be implemented to InlineString without breaking changes. However, adding them to StringExt is breaking change, and it is better to be done at once.

ImmemorConsultrixContrarie commented 4 years ago

Not all of those methods must be required, thus not all of them would be breaking change. For example, first two are as simple as

use std::ops::{Deref, DerefMut};

fn as_str(self: &Self) -> &str
where
    Self: Deref<Target = str>,
{
    &*self
}

fn as_mut_str(self: &mut Self) -> &mut str
where
    Self: DerefMut<Target = str>,
{
    &mut *self
}

into_boxed_str could be done with Into<Box<str>> or with Into<String> the same way.

replace_range may have a default implementation based on drain + insert_str, thus being a non-breaking change.

split_off is a tricky one; should it return a String or an instance of Self? Though, this one could have a default implementation too: From<&str> + truncate.

Due to StringExt: Borrow<str> retain may use char_indices + drain in its default implementation.

Thus there gonna be only two breaking changes: insert_str and drain.

Btw, some of the methods in the trait may become provided instead of required as well: new is just with_capacity(0); as_mut_slice is just a BorrowMut<str> or DerefMut<Target=str>; as_bytes being required is just a joke, because there's Borrow<str> in the trait base itself. Well, as_str being a breaking change becomes a joke too, once you see that Borrow<str>. I really think Borrow<str> must not be in that trait requirements; instead it must be a requirement on per-function basis, so any stringy structure with non-continuous buffer could implement it too.

Anyway, there's still a drain. Drain is an iterator, and that complicates things a lot. To make a drain, quite a lot of code must be written for InlineString struct, then InlinableString enum must get it's own drain iterator and a lot of boilerplate must be written there to get it working and get fold/find/nth/last functions to work in a fast way instead of a default while let Some(_) = iter.next() {} way. And then you still can't write try_fold in stable. Plus, you must put something like type Drain; into the trait, or do it like StringExt: Drainable.

I think, StringExt doesn't need drain the way it's done in the std: fn remove_range<R: std::ops::RangeBound<usize>>(&mut self, range: R) is a much nicer to write, and drain is just

string[range].chars().do_something_with_iterator();
string.remove_range(range);
ImmemorConsultrixContrarie commented 4 years ago

image

Oh, well. However, it's not like drain doesn't have that generic type parameter, so there gonna be some stopsign on the road. Either no drain and replace range, or no trait objects.

(Or a fixed-type range could be used, like start..end only, but that would be ugly.)

ArtemGr commented 4 years ago

split_off is a tricky one; should it return a String or an instance of Self?

Maybe two methods, one returning a String to more easily fit the String-based APIs and another, with an extra suffix in the method name, returning InlinableString for the sake of speed?

ArtemGr commented 4 years ago

Thus there gonna be only two breaking changes: insert_str and drain

Can't the drain be a part of a different trait?