Robbepop / apint

Arbitrary precision integers library.
Other
27 stars 4 forks source link

Clean up the associated constants #54

Closed AaronKutch closed 4 years ago

AaronKutch commented 4 years ago

This makes constants such as digit::ZERO and digit::BITS into associated constants, which makes more sense and eliminates several imports for digit. I also remove redundant functions such as Digit::zero() and Digit::is_all_set(). I decided to keep Digit::is_zero() because of how often it is used and regularized all comparisons of Digit::ZERO.

AaronKutch commented 4 years ago

One more important thing I should mention is that I added

pub(crate) use digit::{
    Digit,
    DoubleDigit,
};

to lib.rs. I believe this is a good change, because of how often these two imports are used.

AaronKutch commented 4 years ago

Almost forgot to run cargo fmt

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-0.9%) to 75.976% when pulling e7153d8822415c81a7581cb4034501f46aca636f on AaronKutch:associated_constants into 21c4c2b2da186bea0a39066042b99d59fd98c225 on Robbepop:master.

codecov-io commented 4 years ago

Codecov Report

Merging #54 into master will increase coverage by 0.42%. The diff coverage is 96.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #54      +/-   ##
==========================================
+ Coverage   75.74%   76.16%   +0.42%     
==========================================
  Files          21       21              
  Lines        5363     5329      -34     
==========================================
- Hits         4062     4059       -3     
+ Misses       1301     1270      -31
Impacted Files Coverage Δ
src/radix.rs 88.88% <ø> (ø) :arrow_up:
src/apint/relational.rs 27.1% <0%> (+1.88%) :arrow_up:
src/apint/utils.rs 79.36% <0%> (ø) :arrow_up:
src/bitpos.rs 100% <100%> (ø) :arrow_up:
src/apint/shift.rs 100% <100%> (ø) :arrow_up:
src/digit.rs 94.74% <100%> (+1.72%) :arrow_up:
src/storage.rs 100% <100%> (ø) :arrow_up:
src/bitwidth.rs 98.59% <100%> (+0.02%) :arrow_up:
src/apint/bitwise.rs 82.63% <100%> (ø) :arrow_up:
src/apint/casting.rs 77.72% <100%> (+1.33%) :arrow_up:
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 21c4c2b...e7153d8. Read the comment docs.

Robbepop commented 4 years ago

I cleaned Travis CI caches and now it works again.

AaronKutch commented 4 years ago

Is it easy to add a CI bot that checks that the crate is formatted?

AaronKutch commented 4 years ago

Rust itself is trying to fix some associated constant related warts: https://github.com/rust-lang/rfcs/pull/2700

AaronKutch commented 4 years ago

I don't see anyway to make this more reviewable. This only changes the way Digit constants are imported and used, and does not change any functionality in functions. The only questionable part is

pub(crate) use digit::{
    Digit,
    DoubleDigit,
};

Previously in my crate reorganization suggestions, I had them used through an info module that shared various small structs. I think we want it to take it one step further by putting it at the crate root like this PR is currently doing, although we could change it in later reorganizations if we want to.

Other small structs that we expose to users are exposed at the crate root, and if we were to hypothetically expose Digit (probably never going to happen because I will have constructors and destructors with slices of u8 all the way to u128 anyway), it would be important enough to be at the crate root.

AaronKutch commented 4 years ago

Its been one month. I am curious about how long it takes you to review PRs.

Robbepop commented 4 years ago

Sorry I forgot about it. :S Will review in the course of today (sunday) or tomorrow.