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

smallvec! macro requires T to be Copy #98

Closed
samnardoni opened this issue May 22, 2018 · 2 comments
Closed

smallvec! macro requires T to be Copy #98

samnardoni opened this issue May 22, 2018 · 2 comments

Comments

@samnardoni
Copy link

@samnardoni samnardoni commented May 22, 2018

The smallvec! macro, when using the smallvec![a,b,c] form, requires that T: Copy.

This is because the macro expands to use SmallVec::from_slice.

    ($($x:expr),*) => ({
        SmallVec::from_slice(&[$($x),*])
    });

Perhaps it could instead expand into something like...

let s = SmallVec::new();
s.push(a);
s.push(b);
s.push(c);
s

... and remove the need for T: Copy?

@jdm
Copy link
Member

@jdm jdm commented May 22, 2018

That seems like a reasonable change. The macro would probably require something like $(s.push($x);*) if you'd like to make a pull request for this.

RReverser added a commit to RReverser/rust-smallvec that referenced this issue Jul 27, 2018
When count of items is smaller or equal than the target inline size, macro will use `SmallVec::push`, but when it's large enough, it passes data on to `vec!` macro for in-place heap allocation and then uses `SmallVec::from_vec`.

This trick gives ~3.5x performance increase on `bench_macro_from_list` compared to `SmallVec::with_capacity`.

Fixes servo#98.
@RReverser
Copy link
Contributor

@RReverser RReverser commented Jul 28, 2018

@samnardoni @jdm I made a PR to fix this issue in a relatively performant manner.

bors-servo added a commit that referenced this issue Aug 4, 2018
Allow smallvec! with non-Copy items

When count of items is smaller or equal than the target inline size, macro will use `SmallVec::push`, but when it's large enough, it passes data on to `vec!` macro for in-place heap allocation and then uses `SmallVec::from_vec`.

This trick gives ~3.5x performance increase on `bench_macro_from_list` compared to `SmallVec::with_capacity`.

Fixes #98.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/107)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Aug 4, 2018
Allow smallvec! with non-Copy items

When count of items is smaller or equal than the target inline size, macro will use `SmallVec::push`, but when it's large enough, it passes data on to `vec!` macro for in-place heap allocation and then uses `SmallVec::from_vec`.

This trick gives ~3.5x performance increase on `bench_macro_from_list` compared to `SmallVec::with_capacity`.

Fixes #98.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/107)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Aug 4, 2018
Allow smallvec! with non-Copy items

When count of items is smaller or equal than the target inline size, macro will use `SmallVec::push`, but when it's large enough, it passes data on to `vec!` macro for in-place heap allocation and then uses `SmallVec::from_vec`.

This trick gives ~3.5x performance increase on `bench_macro_from_list` compared to `SmallVec::with_capacity`.

Fixes #98.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/107)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.