fitzgen / bumpalo

A fast bump allocation arena for Rust
https://docs.rs/bumpalo
Apache License 2.0
1.41k stars 111 forks source link

Add CollectIn, FromIteratorIn traits to allow for easier construction #125

Closed dreamsmasher closed 2 years ago

dreamsmasher commented 2 years ago

Currently, building Bump-parameterized collections from iterators is a little verbose, e.g.

use Bumpalo{Bump, collections::Vec};
let bump = Bump::new();
let xs =  Vec::from_iter_in((0..10), &bump);

while a normal Vec can be constructed directly using .collect(). Having to wrap an entire chain in a method call is ugly for longer expressions, and I find myself using .apply(|iter| Vec::from_iter_in(iter, &bump)) from the apply crate a lot to avoid extra indentation.

This PR adds 2 new traits, FromIteratorIn, and an extension trait CollectIn on all Iterators that allows for simpler constructor of bump-parameterized structures. Currently, Vec and String implement FromIteratorIn, and future collections should try to do the same if possible.

On iterators, CollectIn allows a more chainable syntax for constructing these collections:

let bump = Bump::new();
let xs = (0..10).collect_in::<Vec<i32>>(&bump);

Quickcheck-based tests are also included and passing.

fitzgen commented 2 years ago

(Prior art, for posterity: https://github.com/fitzgen/bumpalo/pull/12)

dreamsmasher commented 2 years ago

(Prior art, for posterity: #12)

Oh wow, didn't see this prior implementation (mainly because I grepped for CollectIn instead of collect_in?). Would you like things to be moved to a collections_traits module still?

fitzgen commented 2 years ago

Would you like things to be moved to a collections_traits module still?

No, this PR is fine, since it isn't modifying the files that are borrowed from std. Thanks!

fitzgen commented 2 years ago

https://github.com/fitzgen/bumpalo/pull/125/checks?check_run_id=3929433788#step:6:80

error[E0277]: can't compare `[i32]` with `std::vec::Vec<i32>`
  --> tests/collect_in.rs:25:14
   |
25 |     bump_ref == &input
   |              ^^ no implementation for `[i32] == std::vec::Vec<i32>`
   |
   = help: the trait `std::cmp::PartialEq<std::vec::Vec<i32>>` is not implemented for `[i32]`
   = note: required because of the requirements on the impl of `std::cmp::PartialEq<&std::vec::Vec<i32>>` for `&[i32]`
dreamsmasher commented 2 years ago

https://github.com/fitzgen/bumpalo/pull/125/checks?check_run_id=3929433788#step:6:80

error[E0277]: can't compare `[i32]` with `std::vec::Vec<i32>`
  --> tests/collect_in.rs:25:14
   |
25 |     bump_ref == &input
   |              ^^ no implementation for `[i32] == std::vec::Vec<i32>`
   |
   = help: the trait `std::cmp::PartialEq<std::vec::Vec<i32>>` is not implemented for `[i32]`
   = note: required because of the requirements on the impl of `std::cmp::PartialEq<&std::vec::Vec<i32>>` for `&[i32]`

Gotta love deref coercion changes. Should be fixed now (tested with cargo +1.44.0 test --features collections,boxed)