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.

Sign up for GitHub

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

Jump to bottom

BigDecimal does not coerce properly during division #2538

Closed
undefinedvalue opened this issue Jan 29, 2015 · 5 comments
Closed

BigDecimal does not coerce properly during division #2538

undefinedvalue opened this issue Jan 29, 2015 · 5 comments
Assignees
@kares
Labels
JRuby 1.7.x JRuby 9000 ruby 2.0-2.2
Milestone
JRuby 1.7.21

Comments

@undefinedvalue
Copy link

BigDecimal raises an error when trying to divide by a non-numeric value, even if it defines #coerce. This is inconsistent with other numeric types and with the other operators in BigDecimal, which properly coerce the value.

Trivial example that reproduces the problem:

class MyNum
  def *(other)
    33
  end

  def /(other)
    99
  end

  def coerce(other)
    [MyNum.new, self]
  end
end
> 10.to_d * MyNum.new
33
> 10.0 / MyNum.new
99
> 10.0.to_d / MyNum.new
MyNum can't be coerced into BigDecimal
@rtyler
Copy link

rtyler commented Jan 30, 2015

@undefinedvalue Can you tell us what version of JRuby you're executing this test case on?

@undefinedvalue
Copy link
Author

1.7.10

@enebo
Copy link
Member

enebo commented Jan 30, 2015

Confirmed on both 1.7 branch and master. 1.9+ from a compay standpoint. Added missing code from original report so it is easier to reproduce below:

require 'bigdecimal'
require 'bigdecimal/util'

class MyNum
  def *(other)
    33
  end

  def /(other)
    99
  end

  def coerce(other)
    [MyNum.new, self]
  end
end

p 10.to_d * MyNum.new
p 10.0 / MyNum.new
p 10.0.to_d / MyNum.new

@philipmw
Copy link

Going from JRuby 1.7.10 to 1.7.11, subtraction breaks. In 1.7.11, both subtraction and division give TypeError: MyNum can't be coerced into BigDecimal, while multiplication and addition work.

@kares kares self-assigned this Jun 3, 2015
kares added a commit to kares/jruby that referenced this issue Jun 3, 2015
@kares
do coercion on * and / (same as MRI has been doing since 1.8) ... fixes
2bfb56d 
jruby#2538
@kares kares added this to the JRuby 1.7.21 milestone Jun 3, 2015
kares added a commit that referenced this issue Jun 4, 2015
@kares
do coercion on * and / (same as MRI has been doing since 1.8) ... fixes
8c2b5f5 
#2538
@kares
Copy link
Member

kares commented Jun 4, 2015

* and / should coerce fine since 8c2b5f5

@kares kares closed this as completed Jun 4, 2015
kares added a commit that referenced this issue Jun 5, 2015
@kares
Merge branch 'jruby-1_7'
7df3e56 
* jruby-1_7: (38 commits)
  basic BigDecimal sub-class test ... all working the same as MRI 1.8/1.9
  avoid deprecated newInstance + one less pattern to match on BigDecimal.new
  [bigdecimal] keep the simplified newInstance backport 1.8 compatible
  [bigdecimal] backport simplified newInstance impl from master
  minor BigDecimal code cleanup
  handle big decimal ** float value calculation (with Java double math) ... since we're now not raising users should get ~ the value they expect (closing #1967)
  static-ize and simplify private helpers
  BigDecimal should not raise error on pow/** with float arg (under 1.9 #1967)
  some more (internal) RubyBigDecimal dry-ing / tidy-ing
  do coercion on * and / (same as MRI has been doing since 1.8) ... fixes #2538
  handle BigDecimal cmp failure compatibly with MRI (fixes #2539)
  pass around context in BigDecimal impl + simplify cmp method's code
  move JI java.mat.BigDecimal test out of Ruby's BigDecimal tests
  [travis-ci] test-extended on jdk8
  if current directory is inside the classloader a spawn jruby process should do the same
  use the new bin stubs from new rubygems
  set ENV['RUBY'] when jruby.home is not regular directory
  Add support for http.nonProxyHosts and rework ENV_JAVA support.
  pik the right class in URLResourceTest
  use new JRuby.create to load script from command line script source
  ...

Conflicts:
	.travis.yml
	core/src/main/java/org/jruby/RubyDir.java
	core/src/main/java/org/jruby/RubyEnumerable.java
	core/src/main/java/org/jruby/RubyEnumerator.java
	core/src/main/java/org/jruby/RubyHash.java
	core/src/main/java/org/jruby/RubyInstanceConfig.java
	core/src/main/java/org/jruby/RubyModule.java
	core/src/main/java/org/jruby/ext/bigdecimal/RubyBigDecimal.java
	core/src/main/java/org/jruby/util/ClasspathResource.java
	core/src/main/java/org/jruby/util/URLResource.java
	core/src/test/java/org/jruby/util/URLResourceTest.java
	lib/ruby/2.0/net/http.rb
	lib/ruby/2.0/uri/generic.rb
	maven/jruby/src/it/runnable/spec/one_spec.rb
	test/test_backquote.rb
	test/test_big_decimal.rb
	test/test_dir.rb
	test/test_file.rb
	test/test_higher_javasupport.rb
	test/test_load.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kares kares

Labels
JRuby 1.7.x JRuby 9000 ruby 2.0-2.2
Projects
None yet
Milestone
JRuby 1.7.21
Development

No branches or pull requests
5 participants
@enebo @rtyler @kares @philipmw @undefinedvalue