akubera / bigdecimal-rs

Arbitrary precision decimal crate for Rust
Other
284 stars 71 forks source link

regression between v0.4.3 and v0.4.4 affecting 32-bit architectures #129

Closed decathorpe closed 4 months ago

decathorpe commented 4 months ago

I tried upgrading the Fedora Linux package for the bigdecimal crate to the latest version, but hit test failures on i686-unknown-linux-gnu.

It is relatively easy to reproduce this on a x86_64 Linux machine:

failures:

---- impl_fmt::test::fmt_boundaries::test_max stdout ----
thread 'impl_fmt::test::fmt_boundaries::test_max' panicked at src/impl_fmt.rs:172:46:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- impl_fmt::test::fmt_boundaries::test_max_multiple_digits stdout ----
thread 'impl_fmt::test::fmt_boundaries::test_max_multiple_digits' panicked at src/impl_fmt.rs:172:46:
called `Option::unwrap()` on a `None` value

---- impl_fmt::test::fmt_boundaries::test_max_scale stdout ----
thread 'impl_fmt::test::fmt_boundaries::test_max_scale' panicked at src/impl_fmt.rs:264:27:
attempt to add with overflow

---- impl_fmt::test::fmt_boundaries::test_min_multiple_digits stdout ----
thread 'impl_fmt::test::fmt_boundaries::test_min_multiple_digits' panicked at src/impl_fmt.rs:264:27:
attempt to add with overflow

---- impl_fmt::test::fmt_boundaries::test_min_scale stdout ----
thread 'impl_fmt::test::fmt_boundaries::test_min_scale' panicked at src/impl_fmt.rs:172:46:
called `Option::unwrap()` on a `None` value

failures:
    impl_fmt::test::fmt_boundaries::test_max
    impl_fmt::test::fmt_boundaries::test_max_multiple_digits
    impl_fmt::test::fmt_boundaries::test_max_scale
    impl_fmt::test::fmt_boundaries::test_min_multiple_digits
    impl_fmt::test::fmt_boundaries::test_min_scale

test result: FAILED. 672 passed; 5 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.19s

Running a git bisect between v0.4.3 ("good") and v0.4.4 ("bad") points to this commit as the cause of the regression: https://github.com/akubera/bigdecimal-rs/commit/ded7f6c7e150589f18d7b70b1e1641892805cbb3

akubera commented 4 months ago

Ok. Those tests are purposefully testing the boundaries, and there’s a case from i64 to usize at line 172

akubera commented 4 months ago

That line wasn't hard to fix (just needed early return), but I end up with an unexpectedly cased 'e':

assertion `left == right` failed
  left: "1E+9223372036854775807"
 right: "1e+9223372036854775807"

So I'll have to track that down.

Also the "add with overflow" errors seem to be coming from another 64-bit assumption, as we're trying to add 4294967294 zeros to the front of a number. I assume there's a saturating add somewhere.

akubera commented 4 months ago

v0.4.5 has been pushed. It should fix the tests. Please let me know here one way or another.

decathorpe commented 4 months ago

Can confirm that tests pass with v0.4.5. Thank you!