Bug #6048

{Unbound}Method#hash doesn't always return the right value

Added by Marc-Andre Lafortune about 3 years ago. Updated about 3 years ago.

[ruby-core:42755]
Status:Closed
Priority:Normal
Assignee:Marc-Andre Lafortune
ruby -v:r34632 Backport:

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é

method_hash.patch Magnifier (4.32 KB) Marc-Andre Lafortune, 02/20/2012 03:19 PM

Associated revisions

Revision 34715
Added by Marc-Andre Lafortune about 3 years ago

  • 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

Revision 34715
Added by Marc-Andre Lafortune about 3 years ago

  • 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

Revision 34717
Added by Nobuyoshi Nakada about 3 years ago

  • proc.c (rb_hash_proc): get wrapped pointer properly. [Bug #6048]

Revision 34717
Added by Nobuyoshi Nakada about 3 years ago

  • proc.c (rb_hash_proc): get wrapped pointer properly. [Bug #6048]

History

#1 Updated by Nobuyoshi Nakada about 3 years ago

Seems fine.

#2 Updated by Nobuyoshi Nakada about 3 years ago

One thing.

rb_method_definition_t* hasn't been used as function interface, rb_method_entry_t* might be preferable.

#3 Updated by Anonymous about 3 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.

#4 Updated by Marc-Andre Lafortune about 3 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

#5 Updated by Nobuyoshi Nakada about 3 years ago

  • Status changed from Closed to Open

#6 Updated by Nobuyoshi Nakada about 3 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]

Also available in: Atom PDF