Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

case expression fails to match a BigDecimal

Description

In 1.5.1 (anywhere before the CLJ-1118 patch), this is the behavior on BigDecimal case matching:

In 1.6 the behavior (post CLJ-1118 patch) has changed:

In 1.6 after CLJ-1118, I expect to see: ("Long" "BigDecimal" "BigDecimal") as they now have the same hash and hasheq.

Cause: The case constants are hashed in the clojure.core/case macro using clojure.core/hash which calls clojure.lang.util/hasheq(). In Compiler.emitExprForHashes(), a call to clojure.lang.Util/hash(). In Clojure 1.5 these hash values are the same (hash of 1.0M == hasheq of 1.0M == 311). In Clojure 1.6, they are different (hash of 1.0M = 311, hasheq of 1.0M = 31).

In any cases where Java's hashCode and Clojure's hasheq return different values, the case statement can fail to do the correct thing.

Approach: Change Compiler.java to use clojure.lang.Util hasheq() to match the case macro use of clojure.core/hash (which calls clojure.lang.Util.hasheq()).

Patch: clj-1301-1.diff

Screened by:

Environment

None

Attachments

2

Activity

Alex Miller January 24, 2014 at 3:46 PM

Alternative patch in the direction of using hashcode/equals instead of hasheq/equiv. Note that this test causes some test failures. This is not yet a candidate patch - further work needs to be done in evaluating this path.

Andy Fingerhut January 16, 2014 at 6:55 PM

One could consider having the default behavior of case to use hasheq and clojure.core/= everywhere, but add a 'fast' option to use hashCode and Java equals.

Alex Miller January 16, 2014 at 6:29 PM

Re Andy's comments above, I walked down that path a bit and built such a patch, however we currently have tests in clojure.test-clojure.control:

which clearly seems to expect Clojure equiv() behavior over Java equals() behavior in case constant matching. So either that is a bad test or this is not a viable approach (it also suggests we could break existing code with this change).

Alex Miller January 16, 2014 at 5:50 PM

Simplified examples.

Alex Miller January 16, 2014 at 3:37 PM

Added better example demonstrating the problem (the specific problem exposed by CLJ-1118).

Completed

Details

Assignee

Reporter

Labels

Approval

Patch

Priority

Affects versions

Fix versions

Created November 23, 2013 at 10:36 PM
Updated January 26, 2014 at 3:03 PM
Resolved January 26, 2014 at 3:03 PM