Open dcsommer opened 4 years ago
Yeah, I can qualify counting bytes vs bits in the text. Typically the size
of integers is specified by bytes, not bits, however. Do you think all
instances of the pattern T::zero().count_zeros()
should be rewritten in
terms of core::mem::size_of::<T>()
instead? I could modify the text to
that effect if you like.
On Sat, Jan 4, 2020 at 9:51 PM Josh Stone notifications@github.com wrote:
@cuviper commented on this pull request.
In src/int.rs https://github.com/rust-num/num-traits/pull/147#discussion_r363071190:
@@ -22,7 +22,7 @@ use {Num, NumCast}; /// /// All
PrimInt
types are expected to be fixed-width binary integers. The width can be queried /// viaT::zero().count_zeros()
. The trait currently lacks a way to query the width at -/// compile-time. +/// compile-time, butcore::mem::size_of::<T>()
can be used in the meantime.Should we say 8 * size_of() so it counts bits?
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rust-num/num-traits/pull/147?email_source=notifications&email_token=AAIIKIFWDUD6NOK6B3HXQZDQ4FYNRA5CNFSM4KB5PIX2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQVUZOY#pullrequestreview-338382011, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIIKIBZLXCYL44UOP5YLMDQ4FYNRANCNFSM4KB5PIXQ .
Typically the size of integers is specified by bytes, not bits, however.
You think so? The bit width is right in the name: i32
, u8
, etc., and C's stdint.h
is similar.
Do you think all instances of the pattern
T::zero().count_zeros()
should be rewritten in terms ofcore::mem::size_of::<T>()
instead?
AFAICS "all" is just this one instance, but sure, I'd be OK with preferring size_of
since it's const
.
I just remembered that in the case of dyn T
we can't use
core::mem::size_of<T>
, so there is definitely a use case for both runtime
and compile-time width calculation. I'll do it in terms of bits too.
On Tue, Jan 7, 2020 at 12:02 PM Josh Stone notifications@github.com wrote:
Typically the size of integers is specified by bytes, not bits, however.
You think so? The bit width is right in the name: i32, u8, etc., and C's stdint.h is similar.
Do you think all instances of the pattern T::zero().count_zeros() should be rewritten in terms of core::mem::size_of::
() instead? AFAICS "all" is just this one instance, but sure, I'd be OK with preferring size_of since it's const.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rust-num/num-traits/pull/147?email_source=notifications&email_token=AAIIKIGSIKUGNGXG2F5GNV3Q4TNVFA5CNFSM4KB5PIX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIKC5EQ#issuecomment-571747986, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIIKIE4ZMMSJQS2A4GOCHDQ4TNVFANCNFSM4KB5PIXQ .
How does dyn T
apply in this context though? We already require PrimInt: Sized
.
I mean when dealing with a PrimInt
trait object, we can't know the size
of the type at compile time. For instance, in code dealing with a Box<dyn PrimInt>
, there isn't a way to use the const fn
core::mem::size_of<T>()
to get the concrete type's width at compile time.
We need to dynamic dispatch on the trait object to calculate it.
On Tue, Jan 7, 2020 at 12:19 PM Josh Stone notifications@github.com wrote:
How does dyn T apply in this context though? We already require PrimInt: Sized.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rust-num/num-traits/pull/147?email_source=notifications&email_token=AAIIKIDT6L5UK4DUHAEHJVDQ4TPVNA5CNFSM4KB5PIX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIKEMEQ#issuecomment-571754002, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIIKIGXTOPD7J6UUMZJL53Q4TPVNANCNFSM4KB5PIXQ .
PrimInt
is not trait object safe -- you can't form any sort of dyn PrimInt
at all.
While the
PrimInt
trait itself doesn't have compile time width functions, there are other ways to find the width of the underlying type. It is useful to present the alternative.