Bug #14573
closedrb_ary_or doesn't check objects hash when the array contains less than SMALL_ARRAY_LEN
Description
Hello,
While debugging a build failure on a 2.5 branch of graphql-ruby (https://github.com/rmosolgo/graphql-ruby/pull/1303) we found out a difference in behavior for the Array's union operator.
On Ruby 2.5:
irb(main):001:0> class X; def initialize(h); @h = h; end; def hash; @h; end; def eql?(o); true; end; end
=> :eql?
irb(main):002:0> a = X.new(1) ; b = X.new(2)
=> #<X:0x00007fe0b00a4130 @h=2>
irb(main):003:0> a.hash
=> 1
irb(main):004:0> b.hash
=> 2
irb(main):005:0> [a] | [b]
=> [#<X:0x00007fe0b00a4158 @h=1>]
I would expect this union would result in two objects. On Ruby 2.3.6 I do get two objects:
irb(main):001:0> class X; def initialize(h); @h = h; end; def hash; @h; end; def eql?(o); true; end; end
=> :eql?
irb(main):002:0> a = X.new(1) ; b = X.new(2)
=> #<X:0x00007f816a80eec0 @h=2>
irb(main):003:0> a.hash
=> 1
irb(main):004:0> b.hash
=> 2
irb(main):005:0> [a] | [b]
=> [#<X:0x00007f816a80eee8 @h=1>, #<X:0x00007f816a80eec0 @h=2>]
I think this change is related to #13884, and it looks like when small arrays (less than SMALL_ARRAY_LEN) are given the object's hash isn't checked. If a bigger array is given we get a correct union:
irb(main):001:0> class X; def initialize(h); @h = h; end; def hash; @h; end; def eql?(o); true; end; end
=> :eql?
irb(main):002:0> a = 20.times.map { |i| X.new(i) }
=> ...
irb(main):003:0> b = 20.times.map { |i| X.new(100 + i) }
=> ...
irb(main):004:0> (a | b).size
=> 40
Thanks,
Willian
Updated by Willian (Willian van der Velde) over 6 years ago
- ruby -v changed from ruby 2.6.0dev (2017-12-27 trunk 61500) [x86_64-darwin17] to ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-darwin17]
Updated by tenderlovemaking (Aaron Patterson) over 6 years ago
I guess I can see how this is a behavior change, but these two objects shouldn't eql?
each other. It seems like a bug in the eql?
implementation of the X
class. These instances won't deal with hash collisions correctly, so I'm not sure why we'd expect union operator to work too.
Updated by lewispb (Lewis Buckley) over 6 years ago
Related to https://bugs.ruby-lang.org/issues/14263. We have posted a note there about our investigation and the inconsistent behaviour between small and larger arrays.
Updated by jeremyevans0 (Jeremy Evans) over 5 years ago
- Related to Bug #14263: Array Intersection does not seem to use hash added
Updated by jeremyevans0 (Jeremy Evans) about 5 years ago
- Status changed from Open to Closed
I don't think this is a bug. If eql?
returns true, hash
must return the same value. Class X
in this example violates that constraint, resulting in undefined behavior. If you fix class X
, the behavior is correct in all ruby versions since at least 1.8:
class X; def initialize(h); @h = h; end; def hash; @h; end; def eql?(o); @h.eql?(o.instance_variable_get(:@h)); end; end