akubera / bigdecimal-rs

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

Trait bound `BigDecimal: From<(BigInt, i64)>` is no longer satisfied after update from `0.2.0` to `0.2.1` #83

Closed piodul closed 3 years ago

piodul commented 3 years ago

This code used to compile with version 0.2.0:

use bigdecimal::BigDecimal;
use num_bigint::{BigInt, Sign}; // num-bigint = "0.3"

fn main() {
    let scale = 5i64;
    let int_value = BigInt::new(Sign::Plus, vec![1]);
    let big_decimal = BigDecimal::from((int_value, scale));
    println!("{:?}", big_decimal);
}

...but no longer does after update to 0.2.1 and fails with the following error:

error[E0277]: the trait bound `BigDecimal: From<(BigInt, i64)>` is not satisfied
 --> src/main.rs:7:23
  |
7 |     let big_decimal = BigDecimal::from((int_value, scale));
  |                       ^^^^^^^^^^^^^^^^ the trait `From<(BigInt, i64)>` is not implemented for `BigDecimal`
  |
  = help: the following implementations were found:
            <BigDecimal as From<(num_bigint::bigint::BigInt, i64)>>
            <BigDecimal as From<i16>>
            <BigDecimal as From<i32>>
            <BigDecimal as From<i64>>
          and 6 others
  = note: required by `from`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.

This broke the build of the scylla crate: https://github.com/scylladb/scylla-rust-driver/issues/286

jplatte commented 3 years ago

This also breaks SQLxes bigdecimal support. The problem is that bigdecimal 0.2.1 upgraded num-bigint, a public dependency, to an semver-incompatible version in a patch release.

@akubera please yank 0.2.1

akubera commented 3 years ago

yanked

akubera commented 3 years ago

So would this be fixed by re-releasing as 0.3.0?

jplatte commented 3 years ago

Yes, if you release as 0.3.0 this won't happen again.

akubera commented 3 years ago

I'm going to roll back num-bigint to "0.3", push that as bigdecimal-0.2.*2*, then bump the number back to 0.4 and release 0.3.0.

Also, I'll expose num-{bigint,traits} as modules in the root of bigdecimal.

Please let me know if there's any objections to this.

jplatte commented 3 years ago

If you have changes independent of the version bump that made 0.2.1 useful then sure, releasing without the num-bigint version bump as 0.2.2 makes sense. I don't think you need to release another 0.2.x to get people un-broken though, yanking should have been enough (the people who have a broken lockfile now are more likely to roll it back or figure out what's wrong and fix it rather than running cargo update again after some time I think).

Also, I'll expose num-{bigint,traits} as modules in the root of bigdecimal.

Just to be clear, you're not expecting this to allow you to bump in patch releases in the future, right?

akubera commented 3 years ago

Just to be clear, you're not expecting this to allow you to bump in patch releases in the future, right?

Absolutely not.

It's more of a convenience so folks don't have to worry about updating in-step with bigdecimal if it is their only dependency which depends on num-bigint.

akubera commented 3 years ago

Two new releases. This time it should work.

piodul commented 3 years ago

I confirm that scylla builds correctly with bigdecimal 0.2.2. Thanks for the quick response!