Clojure

Restore symbol and keyword hashCode to avoid breaking compiled case expressions

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: Release 1.6
  • Fix Version/s: Release 1.6
  • Component/s: None
  • Labels:
    None
  • Environment:
    clojure 1.6.0-beta1
  • Patch:
    Code
  • Approval:
    Ok

Description

case expressions compiled in Clojure 1.5 are broken if run with Clojure 1.6 where hashCode behavior has diverged from hasheq. In particular, Symbol and Keyword fall into this category.

Approach: Cache both hashCode (with 1.5 calculation) and hasheq (new 1.6 calculation) in Symbol and just hasheq in Keyword. In 1.5, these were the same and case expressions compiled with 1.5 will store the old hash calculation. In 1.6, the hashCode of an expression will be used for comparison.

I tested this by AOT compiling a project in clojure 1.5.1 with this function:

(defn check [v]
  (case v
    :k "keyword match"
    'k "symbol match"
    "k" "string match"
    "no match"))

I verified that (check :k) and (check 'v) incorrectly returned "no match" on Clojure 1.6.0-beta1. I then verified that they returned "keyword match" and "symbol match" respectively on Clojure 1.6.0-master with this patch applied.

Patch: clj-1355-v2.patch

Activity

Hide
Alex Miller added a comment -

Add patch that caches a new hash field for both Symbol and Keyword that retains Clojure 1.5 computations.

Show
Alex Miller added a comment - Add patch that caches a new hash field for both Symbol and Keyword that retains Clojure 1.5 computations.
Hide
Alex Miller added a comment -

There is a concern here that we are adding a new int field to every Symbol and Keyword (and keyword holds a symbol, so it's really 2 for each keyword).

Show
Alex Miller added a comment - There is a concern here that we are adding a new int field to every Symbol and Keyword (and keyword holds a symbol, so it's really 2 for each keyword).
Hide
Rich Hickey added a comment -

I don't think we need to cache in keyword, it's just an add

Show
Rich Hickey added a comment - I don't think we need to cache in keyword, it's just an add
Hide
Alex Miller added a comment -

Updated patch to only cache hashCode in symbol and compute in Keyword.

Show
Alex Miller added a comment - Updated patch to only cache hashCode in symbol and compute in Keyword.

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: