Clojure

PersistentQueue's hash function does not match its equality

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: Release 1.4
  • Fix Version/s: Release 1.5
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

There are two issues:

1) PersistentQueue's hash function doesn't match its equiv function:

(def iq (conj clojure.lang.PersistentQueue/EMPTY (Integer. -1)))
(def lq (conj clojure.lang.PersistentQueue/EMPTY (Long. -1)))
[(= iq lq) (= (hash iq) (hash lq))]
;=> [true false]

2) PersistentQueue's hash function doesn't match PersistentVector's hash:

(def q (conj clojure.lang.PersistentQueue/EMPTY 1 2 3))
[(= [1 2 3] q) (= (hash [1 2 3]) (hash q))]
;=> [true false]

Activity

Hide
Philip Potter added a comment -
Show
Philip Potter added a comment - Clojure-dev discussion: https://groups.google.com/d/topic/clojure-dev/ME3-Ke-RbNk/discussion
Nicola Mometto made changes -
Field Original Value New Value
Attachment 001-fix-PersistentQueue-hash.clj [ 11499 ]
Hide
Philip Potter added a comment -

Attached patch 001-make-PersistentQueue-hash-match-equiv.diff, 15/Sep/12.

Show
Philip Potter added a comment - Attached patch 001-make-PersistentQueue-hash-match-equiv.diff, 15/Sep/12.
Philip Potter made changes -
Andy Fingerhut made changes -
Patch Code and Test [ 10002 ]
Hide
Philip Potter added a comment -

Attached 002-make-PersistentQueue-hash-match-equiv.diff, 15/Sep/12. This patch supercedes 001-make-PersistentQueue-hash-match-equiv.diff.

Replaced test code which calls to Util/hasheq with code which calls hash instead.

This patch has a minor conflict with my patch for CLJ-1059, since they both add an interface to PersistentQueue (IHashEq here, List for CLJ-1059).

Show
Philip Potter added a comment - Attached 002-make-PersistentQueue-hash-match-equiv.diff, 15/Sep/12. This patch supercedes 001-make-PersistentQueue-hash-match-equiv.diff. Replaced test code which calls to Util/hasheq with code which calls hash instead. This patch has a minor conflict with my patch for CLJ-1059, since they both add an interface to PersistentQueue (IHashEq here, List for CLJ-1059).
Philip Potter made changes -
Philip Potter made changes -
Assignee Philip Potter [ ppotter ]
Hide
Chouser added a comment -

Thanks for the patch, Philip, it looks good to me.

It really is a pity the implementations of hashCode and hasheq are duplicated. I wonder how important it is to extend Obj? Regardless, that's the approach PersistentQueue was already taking, so changing that would be outside the scope of this ticket anyway.

I can't apply this patch with "git am", but "patch -p 1" works fine. I'm hoping this is a configuration problem on my end, so I'm marking this screened.

Show
Chouser added a comment - Thanks for the patch, Philip, it looks good to me. It really is a pity the implementations of hashCode and hasheq are duplicated. I wonder how important it is to extend Obj? Regardless, that's the approach PersistentQueue was already taking, so changing that would be outside the scope of this ticket anyway. I can't apply this patch with "git am", but "patch -p 1" works fine. I'm hoping this is a configuration problem on my end, so I'm marking this screened.
Chouser made changes -
Approval Screened [ 10004 ]
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Fix Version/s Release 1.5 [ 10150 ]
Stuart Halloway made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: