Skip to content

Commit

Permalink
Showing 5 changed files with 35 additions and 40 deletions.
10 changes: 0 additions & 10 deletions spec/truffle/tags/core/hash/gt_tags.txt

This file was deleted.

10 changes: 0 additions & 10 deletions spec/truffle/tags/core/hash/gte_tags.txt

This file was deleted.

10 changes: 0 additions & 10 deletions spec/truffle/tags/core/hash/lt_tags.txt

This file was deleted.

10 changes: 0 additions & 10 deletions spec/truffle/tags/core/hash/lte_tags.txt

This file was deleted.

35 changes: 35 additions & 0 deletions truffle/src/main/ruby/core/hash.rb
Original file line number Diff line number Diff line change
@@ -36,6 +36,16 @@
class Hash
include Enumerable

def self.contains_all_internal(one, two)
one.all? do |key, value|
begin
value === two.fetch(key)
rescue KeyError
false
end

This comment has been minimized.

Copy link
@pitr-ch

pitr-ch Jun 29, 2016

Member

It would be better to use two.key?(key) && value === two[key] to avoid raising the exception.

This comment has been minimized.

Copy link
@eregon

eregon Jun 29, 2016

Member

=== is the wrong operator.
In MRI, rb_equal is called, but that's actually a.equal?(b) || a == b (Kernel#=== uses rb_equal but the other way around does not hold as === is redefined on other classes).
In Java we have SameOrEqualNode which we could expose as a primitive (but the Ruby code above is likely clearer). Maybe this could be defined in Rubinius::Type.object_same_or_equal(a, b).

This comment has been minimized.

Copy link
@bjfish

bjfish Jun 29, 2016

Author Contributor

@eregon Is the documentation incorrect here? https://github.com/ruby/ruby/blob/trunk/object.c#L78

I think I understand what you mean here. So, the <... methods need only the Object === implementation but not overridden implementations. I'll update this method with these suggestions.

This comment has been minimized.

Copy link
@enebo

enebo Jun 29, 2016

Member

I think the documentation in MRI is correct because it mentions === is overridden in other classes. The implementation is basically what @eregon and the documentation says but it is different for some classes for case/when equality.

This comment has been minimized.

Copy link
@eregon

eregon Jun 29, 2016

Member

That documentation means to document the implementation below. But of course it does not mean rb_equal calls === which would be the semantics with the current code.

This comment has been minimized.

Copy link
@bjfish

bjfish Jun 29, 2016

Author Contributor

@eregon comparison updated here: 5a9505a

end
end

def self.new_from_associate_array(associate_array)
hash = new
associate_array.each do |array|
@@ -95,6 +105,19 @@ def self._constructor_fallback(*args)
# Used internally to get around subclasses redefining #[]=
alias_method :__store__, :[]=

def <(other)
other = Rubinius::Type.coerce_to(other, Hash, :to_hash)
return false if self.size >= other.size
self.class.contains_all_internal(self, other)
end

def <=(other)
other = Rubinius::Type.coerce_to(other, Hash, :to_hash)
return false if self.size > other.size
self.class.contains_all_internal(self, other)
end


def ==(other)
return true if self.equal? other
unless other.kind_of? Hash
@@ -122,6 +145,18 @@ def ==(other)
true
end

def >(other)
other = Rubinius::Type.coerce_to(other, Hash, :to_hash)
return false if self.size <= other.size
self.class.contains_all_internal(other, self)
end

def >=(other)
other = Rubinius::Type.coerce_to(other, Hash, :to_hash)
return false if self.size < other.size
self.class.contains_all_internal(other, self)
end

def assoc(key)
each_item { |e| return e.key, e.value if key == e.key }
end

0 comments on commit 81fdf12

Please sign in to comment.