Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dcsommer
Copy link

@dcsommer dcsommer commented Jan 2, 2020

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.

@@ -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.
Copy link
Member

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?

@dcsommer
Copy link
Author

dcsommer commented Jan 5, 2020 via email

@cuviper
Copy link
Member

cuviper commented Jan 7, 2020

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.

@dcsommer
Copy link
Author

dcsommer commented Jan 7, 2020 via email

@cuviper
Copy link
Member

cuviper commented Jan 7, 2020

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

@dcsommer
Copy link
Author

dcsommer commented Jan 7, 2020 via email

@cuviper
Copy link
Member

cuviper commented Jan 7, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants