num-traits icon indicating copy to clipboard operation
num-traits copied to clipboard

Add documentation of compile-time width function alternative.

Open dcsommer opened this issue 6 years ago • 6 comments

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.

dcsommer avatar Jan 02 '20 00:01 dcsommer

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 [email protected] 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 /// via T::zero().count_zeros(). The trait currently lacks a way to query the width at -/// compile-time. +/// compile-time, but core::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 .

dcsommer avatar Jan 05 '20 06:01 dcsommer

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::<T>() instead?

AFAICS "all" is just this one instance, but sure, I'd be OK with preferring size_of since it's const.

cuviper avatar Jan 07 '20 20:01 cuviper

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 [email protected] 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::<T>() 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 .

dcsommer avatar Jan 07 '20 20:01 dcsommer

How does dyn T apply in this context though? We already require PrimInt: Sized.

cuviper avatar Jan 07 '20 20:01 cuviper

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 [email protected] 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 .

dcsommer avatar Jan 07 '20 22:01 dcsommer

PrimInt is not trait object safe -- you can't form any sort of dyn PrimInt at all.

cuviper avatar Jan 07 '20 22:01 cuviper