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

Array#uniq behavior differs from MRI with BasicObject members #3227

Closed
ryannevell opened this issue Aug 6, 2015 · 2 comments
Closed

Array#uniq behavior differs from MRI with BasicObject members #3227

ryannevell opened this issue Aug 6, 2015 · 2 comments

Comments

@ryannevell
Copy link
Contributor

A class that extends BasicObject and implements ==, eql?, and hash fails to match against others in Array#uniq.

As a result, [a, b].uniq == [a, b], even if:

  • a == b
  • a.eql? b
  • and a.hash == b.hash

Test case follows:

## Make sure that Array#uniq works

require 'test/unit'

# A BasicObject that can be used as a Hash key
class BasicWrap < BasicObject
  attr_reader :x

  def initialize(x)
    @x = x
  end

  def ==(rhs)
    @x == rhs.x
  end
  alias_method :eql?, :==

  def hash
    @x.hash
  end
end

class TestUniq < Test::Unit::TestCase
  def test_uniq_with_fixnum
    a = [3, 2, 1, 4, 1, 2, 3]
    assert(a.uniq == [3, 2, 1, 4])
  end

  def test_uniq_with_struct
    s = Struct.new(:x)
    a = [s.new(3), s.new(2), s.new(1), s.new(4), s.new(1), s.new(2), s.new(3)]
    assert(a.uniq == [s.new(3), s.new(2), s.new(1), s.new(4)])
  end

  def test_uniq_with_basic
    s = BasicWrap
    a = [s.new(3), s.new(2), s.new(1), s.new(4), s.new(1), s.new(2), s.new(3)]
    assert(a.uniq == [s.new(3), s.new(2), s.new(1), s.new(4)])
  end
end

jruby results:

jruby 9.0.1.0-SNAPSHOT (2.2.2) 2015-08-05 ce91559 Java HotSpot(TM) 64-Bit Server VM 25.40-b25 on 1.8.0_40-b26 +jit [darwin-x86_64]

3 tests, 3 assertions, 1 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
66.6667% passed

MRI results:

ruby 2.1.2p95 (2014-05-08 revision 45877) [x86_64-darwin13.0]

3 tests, 3 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
@headius
Copy link
Member

headius commented Aug 6, 2015

Could you turn this into a spec at https://github.com/ruby/rubyspec?

Good find, thanks! We'll fix.

@headius
Copy link
Member

headius commented Aug 6, 2015

Ok, the basic issue is that we don't override Object#hashCode or #equals on RubyBasicObject, so they never attempt to dispatch back into Ruby. Since our Hash impl uses those methods, this is obviously not right.

I have a patch that moves #hashCode and #equals up from RubyObject and introduces "checked" calling of them, but it's a little scary. We may want to keep the RubyObject versions around.

@headius headius closed this as completed in df6cf57 Aug 6, 2015
headius added a commit that referenced this issue Aug 6, 2015
@enebo enebo added this to the JRuby 9.0.1.0 milestone Aug 20, 2015
headius added a commit to ruby/spec that referenced this issue Aug 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants