Bug #6048
closed{Unbound}Method#hash doesn't always return the right value
Description
{Unbound}Method#hash doesn't always return the right value.
map, collect = Array.instance_method(:map), Array.instance_method(:collect)
map.eql?(collect) # => true
map.hash == collect.hash # => false
I'm tempted to think that this is an obvious bug with an obvious solution but let me state:
As per the documentation and the design of hash tables, if two objects are eql?
then they must have the same hash.
Either map
should not be eql?
to collect
or else their hash
should be the same.
As they are identical methods, it is correct that they are eql?
, so hash
must return the same value.
My proposed behavior passes my strict superiority test and is also "straightforward" as I could find no intent for the current behavior.
One solution is to ensure that all aliased methods are defined using rb_define_alias
, which appears to yield the same hash. Another is to compute the hash
correctly. Maybe there is a third approach.
I'm not super confident I took the right approach, but here is a patch for the second one. I think it is more robust and more consistent with how {Unbound}Method.eql?
is implemented. It may also fix other cases of mismatch between eql?
and hash
, I didn't investigate further.
I'd be grateful if someone could review the patch (Koichi?) and let me know if I took the right approach and if I put things in the right place.
Thanks¶
Marc-André
Files
Updated by nobu (Nobuyoshi Nakada) over 12 years ago
Seems fine.
Updated by nobu (Nobuyoshi Nakada) over 12 years ago
One thing.
rb_method_definition_t* hasn't been used as function interface, rb_method_entry_t* might be preferable.
Updated by Anonymous over 12 years ago
Hi,
On Mon, Feb 20, 2012 at 3:24 AM, Nobuyoshi Nakada nobu@ruby-lang.org wrote:
Issue #6048 has been updated by Nobuyoshi Nakada.
One thing.rb_method_definition_t* hasn't been used as function interface, rb_method_entry_t* might be preferable.
Right, I hesitated about this one. Will change.
Thanks for lookup at my patch.
Updated by marcandre (Marc-Andre Lafortune) over 12 years ago
- Status changed from Open to Closed
- % Done changed from 0 to 100
This issue was solved with changeset r34715.
Marc-Andre, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
-
proc.c (method_hash, proc_hash): Fix {Unbound}Method#hash
[Bug #6048]. Isolate hash computation for proc -
internal.h: Declaration for above
-
vm_method.c (rb_method_definition_hash): Computation for
hash part of a method definition -
method.h: Declaration for above
-
test/ruby/test_method.rb: Test for above
Updated by nobu (Nobuyoshi Nakada) over 12 years ago
- Status changed from Closed to Open
Updated by nobu (Nobuyoshi Nakada) over 12 years ago
- Status changed from Open to Closed
This issue was solved with changeset r34717.
Marc-Andre, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
- proc.c (rb_hash_proc): get wrapped pointer properly. [Bug #6048]