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

Deprecate insert_many (and add splice?) #255

Open
mbrubeck opened this issue Jan 8, 2021 · 1 comment
Open

Deprecate insert_many (and add splice?) #255

mbrubeck opened this issue Jan 8, 2021 · 1 comment

Comments

@mbrubeck
Copy link
Collaborator

mbrubeck commented Jan 8, 2021

The insert_many function is complicated, and has had several bugs and vulnerabilities (#96, #208, #252). Unlike most SmallVec methods, it does not correspond to a standard Vec method. Perhaps because of this, it is rarely used. Should we deprecate this method, and remove it in the next major version?

The standard Vec::splice method could be used in place of insert_many, but SmallVec does not yet implement this method. We could add this, possibly by copying the implementation from std::vec, if there is demand for it.

@lopopolo
Copy link

lopopolo commented Jan 8, 2021

@mbrubeck I would like to put in a request for a Vec::splice equivalent API.

I've implemented a crate that exposes several types that would be appropriate to implement Ruby's Array class. This crate has multiple backends, including Vec and SmallVec. Ruby has an Array#[]= API which in one of its forms acts like Vec::splice: replace a subslice of the Array with another slice.

The SmallVec implementation is several times larger and scarier than the Vec implementation.

SmallVec: https://github.com/artichoke/artichoke/blob/ae3fa8adc8e26d74a6ee566e2065b79db5b09fc8/spinoso-array/src/array/smallvec/mod.rs#L1093L1157
Vec: https://github.com/artichoke/artichoke/blob/ae3fa8adc8e26d74a6ee566e2065b79db5b09fc8/spinoso-array/src/array/vec/mod.rs#L1124-L1141

spinoso_array::SmallArray::set_slice might be a starting point for a SmallVec::splice implementation (MIT licensed). It uses no unsafe code.

@mbrubeck mbrubeck changed the title Deprecate insert_many? Deprecate insert_many (and add splice?) Jan 8, 2021
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

No branches or pull requests

2 participants