Skip to content

Commit

Permalink
[Truffle] Implemented TrueClass#hash and FalseClass#hash.
Browse files Browse the repository at this point in the history
@nirvdrum
nirvdrum committed Jan 7, 2015
1 parent ac9c832 commit 206152a
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions core/src/main/java/org/jruby/truffle/nodes/core/KernelNodes.java
View file
Original file line number Diff line number Diff line change
@@ -935,6 +935,11 @@ public int hash(double value) {
return Double.valueOf(value).hashCode();
}

@Specialization
public int hash(boolean value) {
return Boolean.valueOf(value).hashCode();
}

@Specialization
public int hash(RubyBasicObject self) {
return self.hashCode();

5 comments on commit 206152a

@chrisseaton
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be in Ruby? If it's returning a literal there should be literally no difference in generated machine code.

@chrisseaton
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having said that I can't find it in the Rubinius code - but we can always define a true.rb in src/main/ruby/jruby/truffle/core.

@nirvdrum
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find it in Rubinius either. The other issue is the method is defined on either Kernel or Object and inherited. Defining it directly on either TrueClass or FalseClass in Ruby seemed like a violation of the class hierarchy if anyone were to inspect it. I'm open to solutions around that.

@chrisseaton
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like all the basic classes go through the same object_hash primitive, which is what we're doing as well. Not sure if that's to preserve the object hierarchy or not - but that's an interesting thing to keep in mind. Also monkey patching - if you modify Kernel#hash does that modify TrueClass#hash. Maybe not worth worrying too much now either way.

@eregon
Copy link
Member

@eregon eregon commented on 206152a Jan 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, apparently redefining Kernel#hash does redefines the hash of booleans (so this is all good):

[2] pry(main)> o=Object.new
=> #<Object:0x007fa3845b51d8>
[3] pry(main)> o.hash
=> -529235636206727962
[9] pry(main)> module Kernel; def hash; 42; end; end
=> :hash
[10] pry(main)> o.hash
=> 42
[11] pry(main)> false.hash
=> 42
[12] pry(main)> true.hash
=> 42

In any case, test_method_parity should catch this kind of issues.

Please sign in to comment.