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

BigDecimal does not coerce properly during division #2538

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

Comments

Projects
None yet
5 participants
@undefinedvalue

undefinedvalue commented Jan 29, 2015

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

This comment has been minimized.

Show comment
Hide comment
@rtyler

rtyler Jan 30, 2015

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

rtyler commented Jan 30, 2015

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

@undefinedvalue

This comment has been minimized.

Show comment
Hide comment
@undefinedvalue

undefinedvalue commented Jan 30, 2015

1.7.10

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Jan 30, 2015

Member

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
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

This comment has been minimized.

Show comment
Hide comment
@philipmw

philipmw Feb 16, 2015

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.

philipmw commented Feb 16, 2015

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

This comment has been minimized.

Show comment
Hide comment
@kares

kares Jun 4, 2015

Member

* and / should coerce fine since 8c2b5f5

Member

kares commented Jun 4, 2015

* and / should coerce fine since 8c2b5f5

@kares kares closed this Jun 4, 2015

kares added a commit that referenced this issue Jun 5, 2015

Merge branch 'jruby-1_7'
* 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