-
Notifications
You must be signed in to change notification settings - Fork 135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add documentation of compile-time width function alternative. #147
base: master
Are you sure you want to change the base?
Conversation
@@ -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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we say 8 * size_of()
so it counts bits?
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 ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/int.rs
<#147 (comment)>:
> @@ -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
<#147?email_source=notifications&email_token=AAIIKIFWDUD6NOK6B3HXQZDQ4FYNRA5CNFSM4KB5PIX2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQVUZOY#pullrequestreview-338382011>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIIKIBZLXCYL44UOP5YLMDQ4FYNRANCNFSM4KB5PIXQ>
.
|
You think so? The bit width is right in the name:
AFAICS "all" is just this one instance, but sure, I'd be OK with preferring |
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 ***@***.***> 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
<#147?email_source=notifications&email_token=AAIIKIGSIKUGNGXG2F5GNV3Q4TNVFA5CNFSM4KB5PIX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIKC5EQ#issuecomment-571747986>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIIKIE4ZMMSJQS2A4GOCHDQ4TNVFANCNFSM4KB5PIXQ>
.
|
How does |
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 ***@***.***> 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
<#147?email_source=notifications&email_token=AAIIKIDT6L5UK4DUHAEHJVDQ4TPVNA5CNFSM4KB5PIX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIKEMEQ#issuecomment-571754002>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIIKIGXTOPD7J6UUMZJL53Q4TPVNANCNFSM4KB5PIXQ>
.
|
|
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.