ensure len and capacity methods reflect their intentions#4
ensure len and capacity methods reflect their intentions#4neevek wants to merge 2 commits intoasync-email:masterfrom
Conversation
|
The implementation of I opened #5 suitable for a minor bugfix release, but I agree that |
|
@neevek See discussion at chatmail/async-imap#77 (comment), maybe it is not needed to have additional method because poolable vector is supposed to have exactly the same length and capacity all the time. I don't know what is the idea about HashMap though, we definitely cannot pre-fill it with default values as we cannot generate "default keys". @flub Maybe we drop HashMap support completely, is anyone using it? It is inconsistent with what Vec does, because it reallocates capacity while Poolable::capacity() looks at the length. Maybe we better focus this crate at allocating Vec only or even Vec only. |
|
|
||
| fn alloc(size: usize) -> Self { | ||
| vec![T::default(); size] | ||
| Vec::<T>::with_capacity(size) |
There was a problem hiding this comment.
This changes semantics quite a bit and breaks len == capacity invariant mentioned in chatmail/async-imap#77 (comment)
I think this line should be reverted.
There was a problem hiding this comment.
I agree that this completely changes the original semantics, and that indeed will break code that uses the crate.
How about adding another alloc function that only reserves the requested size of memory, and doesn't fill the buffer with default values, which makes len work the way I intended, for example adding a reserve associated function, code that relies on alloc are not affected, and new code can choose to use alloc or reserve.
There was a problem hiding this comment.
If that is okay, we can keep the implementation of the Realloc trait unchanged, and add a Reserve trait that provides a reserve function for poolable objects, or maybe we don't even need to do that since only Vec is used as poolable object.
|
@link2xt I use byte-pool as an alternative to |
No description provided.