Robbepop / apint

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

Issue 42 fixes #56

Closed AaronKutch closed 4 years ago

AaronKutch commented 4 years ago

While I figure out what to do about the BitWidth situation, I decided to go ahead an fix most of #42.

The only question I have now is if we should replace the single bit set/unset/flip functions with a single function set_bit that takes a boolean as an argument.

AaronKutch commented 4 years ago

Also, should I split up the first commit for you, because I originally wanted it to be two separate commits but forgot to commit in the middle, and it is tedious to separate the two?

coveralls commented 4 years ago

Coverage Status

Coverage increased (+1.07%) to 81.913% when pulling 29c7ee448ea18f5ef9a4d1ced2024c1cc5bd941c on AaronKutch:issue-42 into 130949d37601a809d8d463430a94cbb17251a7a6 on Robbepop:master.

codecov-io commented 4 years ago

Codecov Report

Merging #56 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #56   +/-   ##
=======================================
  Coverage   80.47%   80.47%           
=======================================
  Files          23       23           
  Lines        5229     5229           
=======================================
  Hits         4208     4208           
  Misses       1021     1021

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 130949d...fd83c51. Read the comment docs.

Robbepop commented 4 years ago

While I figure out what to do about the BitWidth situation, I decided to go ahead an fix most of #42.

The only question I have now is if we should replace the single bit set/unset/flip functions with a single function set_bit that takes a boolean as an argument.

The set_bit function makes sense to replace set and unset, however, maybe for flip we want to keep it since it does two things (lookup and modification at bit level).

AaronKutch commented 4 years ago

I have changed my mind and want to keep them for symmetry purposes. If I really want to bikeshed, then I would implement bit indexing for ApInts, but that leads right into bitslices and other stuff that will probably come after 0.3.0. I think this is ready.

AaronKutch commented 4 years ago

The CI needs cargo-fmt installed. Do you need to do rustup stuff on caches your end? I actually do not know much about CI in particular.

Also, I noticed that cargo test (no --release) is being used in travis. One cargo test is needed to test for stray overflows, but is a waste of time to run on more than one platform because of how slow the fuzz tester is on debug mode.

Robbepop commented 4 years ago

The CI needs cargo-fmt installed. Do you need to do rustup stuff on caches your end? I actually do not know much about CI in particular.

Also, I noticed that cargo test (no --release) is being used in travis. One cargo test is needed to test for stray overflows, but is a waste of time to run on more than one platform because of how slow the fuzz tester is on debug mode.

Fuzz tests should always be put behind a feature flag so that they only run when the feature flag is enabled since cargo test should by default only ever run fast tests for a nice debug-exec cycle.

AaronKutch commented 4 years ago

I wanted to finish my work on specialized-div-rem before working on apint again, but things are taking longer than expected. I decided to look at this PR again. I think this PR is still good, the only change I just made is to remove the commits about CI, which you are planning on replacing with github actions.

AaronKutch commented 4 years ago

I have added the one and is_one methods back to Int and UInt

codecov-commenter commented 4 years ago

Codecov Report

Merging #56 into master will increase coverage by 1.40%. The diff coverage is 80.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
+ Coverage   80.47%   81.87%   +1.40%     
==========================================
  Files          23       23              
  Lines        5229     5132      -97     
==========================================
- Hits         4208     4202       -6     
+ Misses       1021      930      -91     
Impacted Files Coverage Δ
src/apint/relational.rs 27.10% <0.00%> (+1.88%) :arrow_up:
src/apint/serialization.rs 92.42% <ø> (+0.11%) :arrow_up:
src/lib.rs 100.00% <ø> (ø)
src/int.rs 18.98% <46.87%> (+9.15%) :arrow_up:
src/apint/arithmetic.rs 95.77% <80.00%> (+2.34%) :arrow_up:
src/apint/bitwise.rs 89.23% <85.00%> (-1.50%) :arrow_down:
src/apint/utils.rs 79.42% <94.73%> (+0.06%) :arrow_up:
src/uint.rs 32.35% <95.12%> (+21.32%) :arrow_up:
src/apint/casting.rs 72.84% <100.00%> (-3.56%) :arrow_down:
src/apint/constructors.rs 99.75% <100.00%> (+2.24%) :arrow_up:
... and 23 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 130949d...29c7ee4. Read the comment docs.

Robbepop commented 4 years ago

Thank you for your work @AaronKutch !