Robbepop / apint

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

Fix various trait problems #55

Closed AaronKutch closed 4 years ago

AaronKutch commented 4 years ago

This starts by renaming traits.rs to width.rs to avoid name collisions. I decided in the middle to name the new file std_ops.rs, but I still think it is a better name.

I then simplify all the imports (just changing stuff like crate::bitwidth::BitWidth to crate::BitWidth). I encountered a weird rustfmt bug in constructors.rs, so I just put a #[rustfmt::skip] and TODO on the affected macro.

The next commit gathers creates a new std_ops.rs file which will contain all std::ops functionality except for Drop. I deleted every other impl in the crate because they used inconsistent signatures and used cloning inside them.

The next commit does the same for UInts and Ints. The diff is pretty confusing, but I just indented the impls into a new macro that handles ApInt, UInt, and Int at the same time. No more need to check all duplicate impls around the crate for typos.

The next commit updates the docs and the final commit adds some missing Width implementations.

coveralls commented 4 years ago

Coverage Status

Coverage increased (+5.1%) to 80.867% when pulling e5d3ac8ccc1bc1d7c6e440dd7760927577eb3c53 on AaronKutch:fix_traits into b8f4320dbe1a3a42211f15a9b5d435529ab91fd4 on Robbepop:master.

codecov-io commented 4 years ago

Codecov Report

Merging #55 into master will increase coverage by 5.12%. The diff coverage is 90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
+ Coverage   75.34%   80.47%   +5.12%     
==========================================
  Files          21       22       +1     
  Lines        5333     5228     -105     
==========================================
+ Hits         4018     4207     +189     
+ Misses       1315     1021     -294
Impacted Files Coverage Δ
src/digit.rs 94.24% <ø> (ø) :arrow_up:
src/bitwidth.rs 89.06% <ø> (-0.17%) :arrow_down:
src/storage.rs 100% <ø> (ø) :arrow_up:
src/digit_seq.rs 77.77% <ø> (ø) :arrow_up:
src/apint/bitwise.rs 90.73% <ø> (+8.85%) :arrow_up:
src/apint/relational.rs 25.21% <ø> (ø) :arrow_up:
src/apint/casting.rs 76.39% <ø> (ø) :arrow_up:
src/apint/serde_impl.rs 77.85% <ø> (+0.48%) :arrow_up:
src/apint/arithmetic.rs 93.43% <ø> (+2.72%) :arrow_up:
src/apint/serialization.rs 92.3% <ø> (ø) :arrow_up:
... and 20 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 b8f4320...e5d3ac8. Read the comment docs.

AaronKutch commented 4 years ago

Wow I wasn't expecting you to approve it so quickly. It is ready for merging.

AaronKutch commented 4 years ago

wait I forgot to test for no-std compatibility

AaronKutch commented 4 years ago

If you can I would add to the CI a test that just does cargo build --no-default-features and see if it compiles without errors. Also, I don't think there is a rustfmt test either.

Robbepop commented 4 years ago

If you can I would add to the CI a test that just does cargo build --no-default-features and see if it compiles without errors. Also, I don't think there is a rustfmt test either.

I'd love to see an improved CI. Go ahead but please do this in another PR. I will merge this now. ;) Thanks.