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

Make BigDecimal::limit work for subtraction and division. Fix issue #1615 #4830

Merged
merged 2 commits into from Oct 30, 2017

Conversation

@MrBerg
Copy link
Contributor

@MrBerg MrBerg commented Oct 27, 2017

No description provided.

@kares
kares approved these changes Oct 27, 2017
Copy link
Member

@kares kares left a comment

👍

@@ -1040,7 +1035,9 @@ public IRubyObject op_quo(ThreadContext context, IRubyObject other) {
int pow = len / 4;
int precision = (pow + 1) * 4 * 2;

return divWithScale(context, val, precision);
IRubyObject result = divWithScale(context, val, precision);
if (result.getClass() == RubyBigDecimal.class) return ((RubyBigDecimal) result).setResult();

This comment has been minimized.

@kares

kares Oct 27, 2017
Member

why not an instanceof check?

This comment has been minimized.

@MrBerg

MrBerg Oct 27, 2017
Author Contributor

Paranoia that there might be some hidden RubyBigDecimal subclass with strange behaviour.

Nah, it can totally be changed to instanceof.

This comment has been minimized.

@kares

kares Oct 29, 2017
Member

thx, if you get a chance please adjust - JRuby is quite in control of its sub-classes.
no need for paranoia 😉

This comment has been minimized.

@MrBerg

MrBerg Oct 30, 2017
Author Contributor

Sure, I pushed a fix.

@headius headius added this to the JRuby 9.1.14.0 milestone Oct 30, 2017
@headius
Copy link
Member

@headius headius commented Oct 30, 2017

👍 from me too. I'll merge to master and 9.1.

@headius headius merged commit 850a14b into jruby:master Oct 30, 2017
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
@MrBerg MrBerg deleted the MrBerg:enforce-bigdecimal-limit branch Oct 30, 2017
@headius
Copy link
Member

@headius headius commented Oct 30, 2017

Actually looks like there are other changes to BigDecimal on master that would need to be merged to jruby-9.1 first, like the divWithScale method. I'll try to sort it out.

@headius
Copy link
Member

@headius headius commented Oct 30, 2017

Merged into 9.1 as dc7b498 and 28eb942. The missing commit from master to make it merge nicely was f386054.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.