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 from_const_with_len #301

Closed
jakergrossman opened this issue Jul 5, 2023 · 2 comments
Closed

Add from_const_with_len #301

jakergrossman opened this issue Jul 5, 2023 · 2 comments

Comments

@jakergrossman
Copy link

jakergrossman commented Jul 5, 2023

What are the projects thoughts on adding from_const_and_len version of from_const to be able to create a SmallVec in a const context that does not logically occupy the full inline stack space? Like:

#[cfg_attr(docsrs, doc(cfg(feature = "const_new")))]
#[inline]
pub const fn from_const_and_len(items: [T; N], len: usize) -> Self {
    if len >= N {
        panic!("Expected len to be less than or equal to array size");
    }
    SmallVec {
        capacity: len,
        data: SmallVecData::from_const(MaybeUninit::new(items)),
    }
}

Example Application

The reason this came up is that I'm working on a MIDI library. MIDI uses a variable length quantity (VLQ) type for integers. I'm using a SmallVec<[u8; 4]> as the backing storage so that a VLQ can store 4 bytes inline before spilling onto the heap. I wanted to be able to create a VLQ::ZERO constant to represent 0 stored as a VLQ, which means the backing SmallVec needs to be logically [0] (capacity = 1). But my only option (as far as I am aware) was SmallVec::from_const([0; 4]) which would logically be [0, 0, 0, 0], which is not a valid representation of 0 as a VLQ.

Issues

Unfortunately, panic! in const contexts was only stabilized in Rust 1.57, which as far as I am aware would cause an issue since currently the const_new feature only requires Rust 1.51. I'm not sure how (if at all) this can be mitigated to keep from_const_and_len safe while not changing the version requirements for the const_new feature.

@mbrubeck
Copy link
Collaborator

mbrubeck commented Jul 5, 2023

I'm open to this idea. I would love it if the function could take a slice instead of an array + length, but I don't know whether that will be possible.

Unfortunately, panic! in const contexts was only stabilized in Rust 1.57, which as far as I am aware would cause an issue since currently the const_new feature only requires Rust 1.51.

We could put the new function behind its own feature gate that requires Rust 1.57.

Also note that, as soon as I have time, I will be switching the main development branch to #284 which will be the basis for smallvec 2.0, which gives us a chance to increase the minimum Rust version and make other breaking changes.

@jakergrossman
Copy link
Author

jakergrossman commented Jul 6, 2023

I would love it if the function could take a slice instead of an array + length

I'm not sure how you could specify the logical capacity passing just a slice, though. Maybe I'm missing something.

Edit: Hm I guess you would do something like:

impl<T, const N: usize> SmallVec<[T; N]> {

    // ...

    pub const fn from_const_slice(items: &[T]) -> Self {
        if items.len() >= N {
            panic!("Expected length of slice to be less than or equal to array size");
        }

        SmallVec {
            // ... something here
        }
    }

    // ...

}

I guess I'm not sure how we can constly move a smaller size slice into a bigger array, since we want to move the contiguous slice into the SmallVec.

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