Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Int64 doesn't handle lower bound properly #76

Closed
cakey opened this issue Jan 31, 2018 · 2 comments
Closed

Int64 doesn't handle lower bound properly #76

cakey opened this issue Jan 31, 2018 · 2 comments
Assignees
Labels

Comments

@cakey
Copy link

cakey commented Jan 31, 2018

d := new(decimal.Big).SetRat(new(big.Rat).SetInt64((math.MinInt64)))
fmt.Println(d.Int64())

d2 := new(decimal.Big).SetRat(new(big.Rat).SetInt64((math.MinInt64 + 300)))
fmt.Println(d2.Int64())

d3 := new(decimal.Big).SetRat(new(big.Rat).SetInt64((math.MinInt64 + 350)))
fmt.Println(d3.Int64())
> 0 false
> 0 false
> -9223372036854775000 true

Unless I'm misunderstanding, all 3 should return true?

It looks like it's triggering the u > math.MaxInt64 branch.

I imagine it might be something related to .compact being uint64, but here we care about int64, but I haven't dived in too deep.

I realised it might have something to do with the set precision - if i set precision high enough, then math.MinInt64+1 works correctly, but never math.MinInt64.

Is this intended behaviour?

@ericlagergren
Copy link
Owner

ericlagergren commented Feb 1, 2018

You're correct about precision. Your call to SetRat rounds down based on z's Context. (Essentially, SetRat is computed as z = a / b where a and b are the numerator and denominator of x, respectively.)

You're also correct that branch doesn't take into consideration that the negative half of a 64-bit integer is +1 larger than the positive half.

@ericlagergren ericlagergren self-assigned this Feb 1, 2018
@ericlagergren ericlagergren changed the title bug: Int64() has incorrect results close to math.MinInt64 Int64 doesn't handle lower bound properly Feb 1, 2018
ericlagergren added a commit that referenced this issue Feb 1, 2018
@ericlagergren
Copy link
Owner

Okay. That's fixed. I'm gonna push up a 3.1 which has a handful of bug fixes once Travis CI green lights me. Thanks for reporting them. Your issue made me curious: do you need something like Context.SetRat? I've omitted Context versions of most of the SetX methods, but I'm not opposed to adding them.

ericlagergren added a commit that referenced this issue Feb 6, 2018
Fixes: #76
Updates: #46

Former-commit-id: 719ede1
ericlagergren added a commit that referenced this issue Feb 6, 2018
Updates: #76

Former-commit-id: 8d7c469
ericlagergren added a commit that referenced this issue Feb 6, 2018
Fixes: #76
Updates: #46

Former-commit-id: 1d6ba9e5a075981548170bece262de4675f72ebd [formerly 719ede1]
Former-commit-id: a74d449
ericlagergren added a commit that referenced this issue Feb 6, 2018
Updates: #76

Former-commit-id: 9384ac37e5b502b2f03e3545065e4c7c0d54f8a4 [formerly 8d7c469]
Former-commit-id: 52736a2
ericlagergren added a commit that referenced this issue Mar 6, 2018
Fixes: #76
Updates: #46

Former-commit-id: 1d6ba9e5a075981548170bece262de4675f72ebd [formerly 719ede1]
Former-commit-id: a74d449
ericlagergren added a commit that referenced this issue Mar 6, 2018
Updates: #76

Former-commit-id: 9384ac37e5b502b2f03e3545065e4c7c0d54f8a4 [formerly 8d7c469]
Former-commit-id: 52736a2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants