Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add methods to String which can remove multiple elements at once #570

Closed
wants to merge 2 commits into from
Closed

Add methods to String which can remove multiple elements at once #570

wants to merge 2 commits into from

Conversation

mahkoh
Copy link
Contributor

@mahkoh mahkoh commented Jan 11, 2015

Add a method to String that allows the user to remove more than
one characterat a time while preserving the order of the remaining
characters.

Rendered

@Gankra
Copy link
Contributor

Gankra commented Jan 11, 2015

CC @aturon

This makes sense to me, although adding ever-more methods for this sort of thing is unfortunate. Especially because I forsee someone saying they want this API in iterator form, or just "returns a Vec/String" form.

Perhaps this could be generalized to a drain_range method which returns an iterator that back-shifts the elements when it is dropped? I don't think there should be any notable overhead of calling drain_range(a, b); over remove_range(a, b);

@SimonSapin
Copy link
Contributor

You can implement these methods outside of libstd using based on unsafe code. This would both allow you to use them until they’re included, and help make your proposal more concrete.

Rather than take a pair of usize, the methods could accept the new range syntax. Since they’d need to be generic (with double dispatch) to accept the various range types, they could also accept a single usize and be merged with the existing methods. For example:

impl Vec {
    pub fn remove<R>(&mut self, R) where R: RemoveFromVec {
        R.remove_from(self)
    }
}

pub trait RemoveFromVec {
    fn remove_from(&self, &mut Vec);
}

// Usage: v.remove(i), same as currently
impl RemoveFromVec for usize { /* ... */ }

// Usage: v.remove(i..j)
impl RemoveFromVec for Range<usize> { /* ... */ }

// Usage: v.remove(i..), same as v.truncate(i) (which could be removed?)
impl RemoveFromVec for RangeFrom<usize> { /* ... */ }

// Usage: v.remove(..i)
impl RemoveFromVec for RangeTo<usize> { /* ... */ }

// Usage: v.remove(..), same as v.clear() (which could be removed?)
impl RemoveFromVec for FullRange { /* ... */ }

@Gankra
Copy link
Contributor

Gankra commented Jan 11, 2015

clear(); can already be removed in favour of drain();, which we opted not to do since it would be tricksy. Although I'm not opposed to revisiting that.

@Gankra
Copy link
Contributor

Gankra commented Jan 11, 2015

Also note that remove is currently defined to return T (and panic if OOB). This would require some kind of associated type for the RemoveFromVec trait.

@SimonSapin
Copy link
Contributor

An associated type for the return value could work, but might be "too much magic". So maybe remove_range should not be merged with remove (and return nothing).

@Gankra
Copy link
Contributor

Gankra commented Jan 11, 2015

We could theoretically fold drain into remove by having all of these return an Iterator (except remove(usize), of course).

CC @cgaebel thoughts?

@mahkoh
Copy link
Contributor Author

mahkoh commented Jan 11, 2015

#![feature(unsafe_destructor, slicing_syntax)]

use std::{ptr, mem};
use std::ops::{Range, RangeFrom, RangeTo, FullRange};

pub struct DrainRangeIter<'a, T: 'a> {
    vec: &'a mut Vec<T>,
    next: *mut T,
    next_back: *mut T,
    range_end: *mut T,
    rest: usize,
}

impl<'a, T> Iterator for DrainRangeIter<'a, T> {
    type Item = T;

    fn next(&mut self) -> Option<T> {
        unsafe {
            if self.next == self.next_back {
                None
            } else {
                if mem::size_of::<T>() == 0 {
                    let ret = ptr::read(1u as *mut T);
                    self.next = (self.next as usize + 1) as *mut T;
                    Some(ret)
                } else {
                    let ret = ptr::read(self.next);
                    self.next = self.next.offset(1);
                    Some(ret)
                }
            }
        }
    }

    fn size_hint(&self) -> (uint, Option<uint>) {
        let diff = (self.next_back as uint) - (self.next as uint);
        let size = mem::size_of::<T>();
        let exact = diff / (if size == 0 {1} else {size});
        (exact, Some(exact))
    }
}

impl<'a, T> DoubleEndedIterator for DrainRangeIter<'a, T> {
    fn next_back(&mut self) -> Option<T> {
        unsafe {
            if self.next == self.next_back {
                None
            } else {
                if mem::size_of::<T>() == 0 {
                    self.next_back = (self.next_back as usize - 1) as *mut T;
                    Some(ptr::read(1u as *mut T))
                } else {
                    self.next_back = self.next_back.offset(-1);
                    Some(ptr::read(self.next))
                }
            }
        }
    }
}

#[unsafe_destructor]
impl<'a, T> Drop for DrainRangeIter<'a, T> {
    fn drop(&mut self) {
        for _x in *self { }
        let ptr = self.vec.as_mut_ptr();
        let len = self.vec.len();
        unsafe {
            let dest = ptr.offset(len as isize);
            ptr::copy_memory(dest, self.range_end, self.rest);
            self.vec.set_len(len + self.rest);
        }
    }
}

pub trait DrainFromVec {
    fn drain_from<'a, T>(&self, vec: &'a mut Vec<T>) -> DrainRangeIter<'a, T>;
}

impl DrainFromVec for Range<usize> {
    fn drain_from<'a, T>(&self, vec: &'a mut Vec<T>) -> DrainRangeIter<'a, T> {
        assert!(self.start <= self.end);
        assert!(self.end <= vec.len());

        let ptr = vec.as_mut_ptr();
        let len = vec.len();

        unsafe {
            vec.set_len(self.start);
            let (next, range_end) = if mem::size_of::<T>() == 0 {
                ((ptr as usize + self.start) as *mut T, (ptr as usize + self.end) as *mut T)
            } else {
                (ptr.offset(self.start as isize), ptr.offset(self.end as isize))
            };
            DrainRangeIter {
                vec: vec,
                next: next,
                next_back: range_end,
                range_end: range_end,
                rest: len - self.end,
            }
        }
    }
}

impl DrainFromVec for RangeFrom<usize> {
    fn drain_from<'a, T>(&self, vec: &'a mut Vec<T>) -> DrainRangeIter<'a, T> {
        (self.start..vec.len()).drain_from(vec)
    }
}

impl DrainFromVec for RangeTo<usize> {
    fn drain_from<'a, T>(&self, vec: &'a mut Vec<T>) -> DrainRangeIter<'a, T> {
        (0..self.end).drain_from(vec)
    }
}

impl DrainFromVec for FullRange {
    fn drain_from<'a, T>(&self, vec: &'a mut Vec<T>) -> DrainRangeIter<'a, T> {
        (0..vec.len()).drain_from(vec)
    }
}

pub trait DrainRange<T> {
    fn drain_range<'a, R>(&'a mut self,
                          r: R) -> DrainRangeIter<'a, T> where R: DrainFromVec;
}

impl<T> DrainRange<T> for Vec<T> {
    fn drain_range<'a, R>(&'a mut self,
                           r: R) -> DrainRangeIter<'a, T> where R: DrainFromVec {
        r.drain_from(self)
    }
}

fn main() {
    let mut s = b"abcdefghijklm".to_vec();
    for b in s.drain_range(..5) {
        println!("{}", b as char);
    }
    println!("{:?}", s);
}

Naturally this doesn't work for String.

@mahkoh mahkoh changed the title Add methods to String and Vec which can remove multiple elements at once Add methods to String which can remove multiple elements at once Jan 12, 2015
@mahkoh
Copy link
Contributor Author

mahkoh commented Jan 12, 2015

The vector part has been moved to #574.

@aturon aturon self-assigned this Jan 22, 2015
@aturon
Copy link
Member

aturon commented Feb 16, 2015

Closing in favor of the RFCs.

@aturon aturon closed this Feb 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants