Clojure

case expression fails to match a BigDecimal

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Blocker Blocker
  • Resolution: Completed
  • Affects Version/s: Release 1.5, Release 1.6
  • Fix Version/s: Release 1.6
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Incomplete

Description

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

user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (map t [1 1.0M 1.00M])
("Long" "BigDecimal" "none")

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

user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (map t [1 1.0M 1.00M])
("Long" "none" "none")

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:

  1. case-alt.patch
    24/Jan/14 9:46 AM
    3 kB
    Alex Miller
  2. clj-1301-1.diff
    23/Nov/13 5:00 PM
    2 kB
    Andy Fingerhut

Activity

Andy Fingerhut made changes -
Field Original Value New Value
Description The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash(), which is Java's hashCode(), plus a case for returning 0 for null.

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

{code}
user=> (defn test-fn [x]
  #_=> (case x
  #_=> (byte -5) "byte -5"
  #_=> "other"))
#'user/test-fn
user=> (test-fn 4)
"other"
user=> (test-fn (byte -5))
"other"
user=> (test-fn -5)
"byte -5"
user=> (.hashCode -5)
4
user=> (.hashCode (byte -5))
-5
user=> (hash -5)
4
user=> (hash (byte -5))
4
{code}

The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash(), which is Java's hashCode(), plus a case for returning 0 for null.

In any cases where Java's hashCode and Clojure's hasheq return different values, the case statement can fail to do the correct thing.
Hide
Andy Fingerhut added a comment -

Patch clj-1301-1.diff modifies Compiler.java so that case* statements use hasheq on the test expression value, rather than Java's hashCode. It also adds a test case that currently fails with latest Clojure master, but passes with the patch.

Show
Andy Fingerhut added a comment - Patch clj-1301-1.diff modifies Compiler.java so that case* statements use hasheq on the test expression value, rather than Java's hashCode. It also adds a test case that currently fails with latest Clojure master, but passes with the patch.
Andy Fingerhut made changes -
Attachment clj-1301-1.diff [ 12485 ]
Hide
Andy Fingerhut added a comment -

This bug is also the root cause for the recent failures of tests for the test.generative library.

Show
Andy Fingerhut added a comment - This bug is also the root cause for the recent failures of tests for the test.generative library.
Alex Miller made changes -
Approval Triaged [ 10120 ]
Alex Miller made changes -
Priority Major [ 3 ] Critical [ 2 ]
Alex Miller made changes -
Description Reproduction test case:

{code}
user=> (defn test-fn [x]
  #_=> (case x
  #_=> (byte -5) "byte -5"
  #_=> "other"))
#'user/test-fn
user=> (test-fn 4)
"other"
user=> (test-fn (byte -5))
"other"
user=> (test-fn -5)
"byte -5"
user=> (.hashCode -5)
4
user=> (.hashCode (byte -5))
-5
user=> (hash -5)
4
user=> (hash (byte -5))
4
{code}

The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash(), which is Java's hashCode(), plus a case for returning 0 for null.

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

{code}
user> (defn test-fn [x] (case x (byte -5) "byte -5" "other"))
#'user/test-fn
user> (test-fn 4)
"other"
user> (map test-fn [4 (byte -5) -5])
("other" "other" "byte -5")
user> (map #(.hashCode %) [4 (byte -5) -5])
(4 -5 4)
user> (map hash [4 (byte -5) 5])
(4 4 5)
{code}

The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash(), which is Java's hashCode(), plus a case for returning 0 for null.

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

*Patch:* clj-1301-1.diff

*Screened by:*
Andy Fingerhut made changes -
Patch Code and Test [ 10002 ]
Rich Hickey made changes -
Approval Triaged [ 10120 ] Vetted [ 10003 ]
Hide
Alex Miller added a comment -

Putting in 1.6 release per Rich.

Show
Alex Miller added a comment - Putting in 1.6 release per Rich.
Alex Miller made changes -
Fix Version/s Release 1.6 [ 10157 ]
Alex Miller made changes -
Description Reproduction test case:

{code}
user> (defn test-fn [x] (case x (byte -5) "byte -5" "other"))
#'user/test-fn
user> (test-fn 4)
"other"
user> (map test-fn [4 (byte -5) -5])
("other" "other" "byte -5")
user> (map #(.hashCode %) [4 (byte -5) -5])
(4 -5 4)
user> (map hash [4 (byte -5) 5])
(4 4 5)
{code}

The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash(), which is Java's hashCode(), plus a case for returning 0 for null.

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

*Patch:* clj-1301-1.diff

*Screened by:*
Case expressions select the wrong branch when hash != hasheq.

{code}
user> (defn test-fn [x] (case x (byte -5) "byte -5" "other"))
#'user/test-fn
user> (test-fn 4)
"other"
user> (map test-fn [4 (byte -5) -5])
("other" "other" "byte -5")
;; EXPECT: ("other" "byte -5" "other")
user> (map #(.hashCode %) [4 (byte -5) -5])
(4 -5 4)
user> (map hash [4 (byte -5) 5])
(4 4 5)
{code}

*Cause:* The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash(), which is Java's hashCode(), plus a case for returning 0 for null.

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:*
Alex Miller made changes -
Description Case expressions select the wrong branch when hash != hasheq.

{code}
user> (defn test-fn [x] (case x (byte -5) "byte -5" "other"))
#'user/test-fn
user> (test-fn 4)
"other"
user> (map test-fn [4 (byte -5) -5])
("other" "other" "byte -5")
;; EXPECT: ("other" "byte -5" "other")
user> (map #(.hashCode %) [4 (byte -5) -5])
(4 -5 4)
user> (map hash [4 (byte -5) 5])
(4 4 5)
{code}

*Cause:* The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash(), which is Java's hashCode(), plus a case for returning 0 for null.

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:*
Case expressions select the wrong branch when hash != hasheq because case expression uses different hash strategy for the constant values (in the case macro) vs the hash strategy for the expression (in the Compiler).

{code}
user> (defn test-fn [x] (case x (byte -5) "byte -5" "other"))
#'user/test-fn
user> (test-fn 4)
"other"
user> (map test-fn [4 (byte -5) -5])
("other" "other" "byte -5")
;; EXPECT: ("other" "byte -5" "other")
user> (map #(.hashCode %) [4 (byte -5) -5])
(4 -5 4)
user> (map hash [4 (byte -5) 5])
(4 4 5)
{code}

*Cause:* The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash(), which is Java's hashCode(), plus a case for returning 0 for null.

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:*
Summary Clojure compiler uses hashCode() on case* statement values, but core.clj uses hasheq() case expression uses different hash strategy for constant values vs expression
Alex Miller made changes -
Description Case expressions select the wrong branch when hash != hasheq because case expression uses different hash strategy for the constant values (in the case macro) vs the hash strategy for the expression (in the Compiler).

{code}
user> (defn test-fn [x] (case x (byte -5) "byte -5" "other"))
#'user/test-fn
user> (test-fn 4)
"other"
user> (map test-fn [4 (byte -5) -5])
("other" "other" "byte -5")
;; EXPECT: ("other" "byte -5" "other")
user> (map #(.hashCode %) [4 (byte -5) -5])
(4 -5 4)
user> (map hash [4 (byte -5) 5])
(4 4 5)
{code}

*Cause:* The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash(), which is Java's hashCode(), plus a case for returning 0 for null.

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:*
Case expressions select the wrong branch when hash != hasheq because case expression uses different hash strategy for the constant values (in the case macro) vs the hash strategy for the expression (in the Compiler).

{code}
user> (defn test-fn [x] (case x (byte -5) "byte -5" "other"))
#'user/test-fn
user> (test-fn 4)
"other"
user> (map test-fn [4 (byte -5) -5])
("other" "other" "byte -5")
;; EXPECT: ("other" "byte -5" "other")
user> (map #(.hashCode %) [4 (byte -5) -5])
(4 -5 4)
user> (map hash [4 (byte -5) 5])
(4 4 5)
{code}

*Cause:* The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash() (essentially Java's hashCode()).

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:*
Alex Miller made changes -
Summary case expression uses different hash strategy for constant values vs expression case expression uses different hash fn for constant values vs expression
Alex Miller made changes -
Description Case expressions select the wrong branch when hash != hasheq because case expression uses different hash strategy for the constant values (in the case macro) vs the hash strategy for the expression (in the Compiler).

{code}
user> (defn test-fn [x] (case x (byte -5) "byte -5" "other"))
#'user/test-fn
user> (test-fn 4)
"other"
user> (map test-fn [4 (byte -5) -5])
("other" "other" "byte -5")
;; EXPECT: ("other" "byte -5" "other")
user> (map #(.hashCode %) [4 (byte -5) -5])
(4 -5 4)
user> (map hash [4 (byte -5) 5])
(4 4 5)
{code}

*Cause:* The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash() (essentially Java's hashCode()).

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:*
Case expressions select the wrong branch when hash != hasheq because case expression uses different hash strategy for the constant values (in the case macro) vs the hash strategy for the expression (in the Compiler).

{code}
user> (defn test-fn [x] (case x (byte -5) "byte -5" "other"))
#'user/test-fn
user> (test-fn 4)
"other"
user> (map test-fn [4 (byte -5) -5])
("other" "other" "byte -5")
;; EXPECT: ("other" "byte -5" "other")
user> (map #(.hashCode %) [4 (byte -5) -5])
(4 -5 4)
user> (map hash [4 (byte -5) 5])
(4 4 5)
{code}

*Cause:* The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash() (essentially Java's hashCode()).

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:* Alex Miller - I think the patch does what it says above and the cause is accurate. My only remaining question is to verify that case should be comparing the Clojure hashcode and not the Java hashcode. This seems right but an alternative patch would be to make the case macro use Java hashcode instead.
Alex Miller made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
Hide
Alex Miller added a comment -

Andy, I talked to Rich and the conclusion was that we should make the opposite change here such that the case macro should route to the Java hashcode version clojure.lang.util.hash() and the Compiler should be left as is. Can you update the patch?

Show
Alex Miller added a comment - Andy, I talked to Rich and the conclusion was that we should make the opposite change here such that the case macro should route to the Java hashcode version clojure.lang.util.hash() and the Compiler should be left as is. Can you update the patch?
Hide
Alex Miller added a comment -

And in case you were wondering, the reason is that the Java hashcode is generally faster (case is all about speed) and there are easy opportunities for you to properly cast your expression and/or case constants (where as the situations with collections where boxing is difficult to fix generically, that is not true).

Show
Alex Miller added a comment - And in case you were wondering, the reason is that the Java hashcode is generally faster (case is all about speed) and there are easy opportunities for you to properly cast your expression and/or case constants (where as the situations with collections where boxing is difficult to fix generically, that is not true).
Hide
Andy Fingerhut added a comment -

Alex, unless I am missing something, changing case to use Java's hashCode() would also require changing its current equality comparison from Clojure = (aka equiv()) to something consistent with hashCode(), which I think must be Java's equals().

Such a change would mean that all of the things that are = but not equals() will not match each other in a case statement, e.g. a case value of (Integer. 5) will not match a (Long. 5) value to compare against in a case branch.

Is that really what is desired here? I almost hesitate to create such a patch, for fear it might be committed

Show
Andy Fingerhut added a comment - Alex, unless I am missing something, changing case to use Java's hashCode() would also require changing its current equality comparison from Clojure = (aka equiv()) to something consistent with hashCode(), which I think must be Java's equals(). Such a change would mean that all of the things that are = but not equals() will not match each other in a case statement, e.g. a case value of (Integer. 5) will not match a (Long. 5) value to compare against in a case branch. Is that really what is desired here? I almost hesitate to create such a patch, for fear it might be committed
Hide
Alex Miller added a comment -

Based on discussion comments, move back to Incomplete until we resolve.

Show
Alex Miller added a comment - Based on discussion comments, move back to Incomplete until we resolve.
Alex Miller made changes -
Approval Screened [ 10004 ] Incomplete [ 10006 ]
Hide
Alex Miller added a comment -

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

Show
Alex Miller added a comment - Added better example demonstrating the problem (the specific problem exposed by CLJ-1118).
Alex Miller made changes -
Description Case expressions select the wrong branch when hash != hasheq because case expression uses different hash strategy for the constant values (in the case macro) vs the hash strategy for the expression (in the Compiler).

{code}
user> (defn test-fn [x] (case x (byte -5) "byte -5" "other"))
#'user/test-fn
user> (test-fn 4)
"other"
user> (map test-fn [4 (byte -5) -5])
("other" "other" "byte -5")
;; EXPECT: ("other" "byte -5" "other")
user> (map #(.hashCode %) [4 (byte -5) -5])
(4 -5 4)
user> (map hash [4 (byte -5) 5])
(4 4 5)
{code}

*Cause:* The code generated by macro clojure.core/case uses clojure.core/hash to hash test-constant values, which is the same as clojure.lang.Util method hasheq().

The code emitted in Compiler.java for case* expressions calls method clojure.lang.Util method hash() (essentially Java's hashCode()).

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:* Alex Miller - I think the patch does what it says above and the cause is accurate. My only remaining question is to verify that case should be comparing the Clojure hashcode and not the Java hashcode. This seems right but an alternative patch would be to make the case macro use Java hashcode instead.
In 1.5.1 (anywhere before the CLJ-1118 patch), the is the behavior on BigDecimal case matching:

{code}
user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (t 1)
"Long"
user=> (t 1.0M)
"BigDecimal"
user=> (t 1.00M)
"none"
user=> (t 5)
"none"
{code}

In 1.6 the behavior (post CLJ-1118 patch specifically) has changed in unexpected ways:

{code}
user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (t 1)
"Long"
user=> (t 1.0M)
"none"
user=> (t 1.00M)
"none"
{code}

With the CLJ-1118, I would expect to see both 1.0M *and* 1.00M return "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:*
Summary case expression uses different hash fn for constant values vs expression case expression fails to match a BigDecimal
Hide
Alex Miller added a comment -

Simplified examples.

Show
Alex Miller added a comment - Simplified examples.
Alex Miller made changes -
Description In 1.5.1 (anywhere before the CLJ-1118 patch), the is the behavior on BigDecimal case matching:

{code}
user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (t 1)
"Long"
user=> (t 1.0M)
"BigDecimal"
user=> (t 1.00M)
"none"
user=> (t 5)
"none"
{code}

In 1.6 the behavior (post CLJ-1118 patch specifically) has changed in unexpected ways:

{code}
user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (t 1)
"Long"
user=> (t 1.0M)
"none"
user=> (t 1.00M)
"none"
{code}

With the CLJ-1118, I would expect to see both 1.0M *and* 1.00M return "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:*
In 1.5.1 (anywhere before the CLJ-1118 patch), the is the behavior on BigDecimal case matching:

{code}
user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (map t [1 1.0M 1.00M])
("Long" "BigDecimal" "none")
{code}

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

{code}
user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (map t [1 1.0M 1.00M])
("Long" "none" "none")
{code}

With the CLJ-1118, I would 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:*
Alex Miller made changes -
Description In 1.5.1 (anywhere before the CLJ-1118 patch), the is the behavior on BigDecimal case matching:

{code}
user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (map t [1 1.0M 1.00M])
("Long" "BigDecimal" "none")
{code}

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

{code}
user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (map t [1 1.0M 1.00M])
("Long" "none" "none")
{code}

With the CLJ-1118, I would 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:*
In 1.5.1 (anywhere before the CLJ-1118 patch), this is the behavior on BigDecimal case matching:

{code}
user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (map t [1 1.0M 1.00M])
("Long" "BigDecimal" "none")
{code}

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

{code}
user=> (defn t [v] (case v 1 "Long" 1.0M "BigDecimal" "none"))
#'user/t
user=> (map t [1 1.0M 1.00M])
("Long" "none" "none")
{code}

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:*
Hide
Alex Miller added a comment -

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:

(testing "test number equivalence"
    (is (= :1 (case 1N 1 :1 :else))))

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).

Show
Alex Miller added a comment - 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:
(testing "test number equivalence"
    (is (= :1 (case 1N 1 :1 :else))))
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).
Hide
Andy Fingerhut added a comment -

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.

Show
Andy Fingerhut added a comment - 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 made changes -
Priority Critical [ 2 ] Blocker [ 1 ]
Hide
Alex Miller added a comment -

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.

Show
Alex Miller added a comment - 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.
Alex Miller made changes -
Attachment case-alt.patch [ 12718 ]
Rich Hickey made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]

People

Vote (2)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: