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

feat: Add splice #195

Closed
wants to merge 2 commits into from
Closed

feat: Add splice #195

wants to merge 2 commits into from

Conversation

Marwes
Copy link
Contributor

@Marwes Marwes commented Jan 8, 2020

To mirror https://doc.rust-lang.org/alloc/vec/struct.Vec.html#method.splice .

The implementation is a straight copy of Vecs implementation, only
altered to call len()/reserve() instead of accessing the internal
len and buf field respectively as those do not exist in SmallVec.
An extra type parameter were also required for Drain.

To mirror https://doc.rust-lang.org/alloc/vec/struct.Vec.html#method.splice .

The implementation is a straight copy of `Vec`s implementation, only
altered to call `len()/reserve()` instead of accessing the internal
`len` and `buf` field respectively as those do not exist in `SmallVec`.
An extra type parameter were also required for `Drain`.
Copy link
Member

@emilio emilio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't dug through all the code yet, but this needs a bunch of tests for the relevant edge cases, at the very least.

@Marwes
Copy link
Contributor Author

Marwes commented Jan 8, 2020

Copied over the tests for Vec::splice, those don't test the additional edge conditions that SmallVec has but its a start.

@emilio
Copy link
Member

emilio commented Jan 9, 2020

Seems there's something wrong as tests are failing. The error looks legit:

error: Miri evaluation error: Memory access failed: pointer must be in-bounds at offset 24, but is outside bounds of allocation 25122 which has size 20
    --> /home/travis/.rustup/toolchains/nightly-2020-01-08-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/intrinsics.rs:1586:5
     |
1586 |     copy(src, dst, count)
     |     ^^^^^^^^^^^^^^^^^^^^^ Miri evaluation error: Memory access failed: pointer must be in-bounds at offset 24, but is outside bounds of allocation 25122 which has size 20
     |
     = note: inside call to `std::intrinsics::copy::<i32>` at /home/travis/build/servo/rust-smallvec/lib.rs:360:9
     = note: inside call to `smallvec::Drain::<[i32; 1]>::move_tail` at /home/travis/build/servo/rust-smallvec/lib.rs:302:17
     = note: inside call to `<smallvec::Splice<std::iter::Cloned<std::slice::Iter<i32>>, [i32; 1]> as std::ops::Drop>::drop` at /home/travis/.rustup/toolchains/nightly-2020-01-08-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/ptr/mod.rs:184:1
note: inside call to `std::ptr::real_drop_in_place::<smallvec::Splice<std::iter::Cloned<std::slice::Iter<i32>>, [i32; 1]>> - shim(Some(smallvec::Splice<std::iter::Cloned<std::slice::Iter<i32>>, [i32; 1]>))` at tests/splice.rs:7:38
    --> tests/splice.rs:7:38
     |
7    |     v.splice(2..4, a.iter().cloned());
     |                                      ^
note: inside call to `test_splice` at tests/splice.rs:4:1
    --> tests/splice.rs:4:1
     |
4    | / fn test_splice() {
5    | |     let mut v: SmallVec<[_; 1]> = smallvec![1, 2, 3, 4, 5];
6    | |     let a = [10, 11, 12];
7    | |     v.splice(2..4, a.iter().cloned());
...    |
10   | |     assert_eq!(&v[..], &[1, 20, 11, 12, 5]);
11   | | }
     | |_^

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.

None yet

3 participants