You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Note: after writing all of this below, I noticed std::slice::chunks also uses usize and panics when provided a value that is 0, which is really unfortunate, IMO. Of course, we won't be able to change the std lib and we should keep symmetry between Streams and the std lib, but I'm still interested what you think of this.
Issue
I've noticed that the methods chunks and ready_chunks take a usize as an arg for the capacity.
This seems to be a good arg type, because other "capacity" methods in the std lib take usize as well (e.g. Vec::with_capacity) and so there is symmetry between them.
However, there is a huge pitfall and it is actually not symmetric: chunks and ready_chunks will panic, when passed in a usize of 0 (according to docs: This method will panic if capacity is zero.).
The other capacity methods (e.g. on Vec in std) do not panic, when provided with the value 0 (which is sensible, because one can have a Vec that has no allocations).
Proposal
Instead of using usize in the argument type, we should rather use NonZeroUsize, because it guarantees a non-zero value. Note that internally we can still use usize (to make it easier to interact with other methods, such as len() etc.) - this proposed change will only affect the API itself.
use std::error::Error;use core::num::NonZeroUsize;fnmain() -> Result<(),Box<dynError>>{let cap_untrusted = 1usize;// imagine that this value comes from "somewhere" and could be 0let my_chunk = MyChunk::with_capacity(cap_untrusted.try_into()?);println!("{my_chunk:#?}");Ok(())}#[derive(Debug)]#[allow(dead_code)]structMyChunk{cap:usize,// internally, we are free to use usize, so that we have symmetry with e.g. `Vec`...//...}implMyChunk{// ...but our API is more restrictive and won't accept a value that is 0pubfnwith_capacity(cap:NonZeroUsize) -> Self{Self{cap: cap.get()}}}
Related
The following issues might be related and should be considered, when implementing this proposal: #2492 especially this comment
BREAKING
This is a breaking change, because the public API changes.
Hey folks,
Note: after writing all of this below, I noticed std::slice::chunks also uses usize and panics when provided a value that is 0, which is really unfortunate, IMO. Of course, we won't be able to change the std lib and we should keep symmetry between
Stream
s and the std lib, but I'm still interested what you think of this.Issue
I've noticed that the methods
chunks
andready_chunks
take ausize
as an arg for the capacity.This seems to be a good arg type, because other "capacity" methods in the std lib take
usize
as well (e.g.Vec::with_capacity
) and so there is symmetry between them.However, there is a huge pitfall and it is actually not symmetric:
chunks
andready_chunks
will panic, when passed in a usize of 0 (according to docs: This method will panic ifcapacity
is zero.).The other
capacity
methods (e.g. onVec
in std) do not panic, when provided with the value 0 (which is sensible, because one can have a Vec that has no allocations).Proposal
Instead of using
usize
in the argument type, we should rather useNonZeroUsize
, because it guarantees a non-zero value. Note that internally we can still useusize
(to make it easier to interact with other methods, such aslen()
etc.) - this proposed change will only affect the API itself.Here is an exemplary playground to show the general idea:
Related
The following issues might be related and should be considered, when implementing this proposal:
#2492 especially this comment
BREAKING
This is a breaking change, because the public API changes.
The text was updated successfully, but these errors were encountered: