CLJS-3425: Incorrect handling of ##NaN with min/max (#282)

- prefix inlining min/max as unchecked
- fix min and max fns to do what Clojure does

Author: swannodette

src/main/cljs/cljs/core.cljs

@@ -1608,7 +1608,7 @@ reduces them without incurring seq initialization"
        -1
        (loop [idx (cond
                     (pos? start) start
-                    (neg? start) (max 0 (+ start len))
+                    (neg? start) (unchecked-max 0 (+ start len))
                     :else start)]
          (if (< idx len)
            (if (= (nth coll idx) x)
@@ -1624,7 +1624,7 @@ reduces them without incurring seq initialization"
     (if (zero? len)
       -1
       (loop [idx (cond
-                   (pos? start) (min (dec len) start)
+                   (pos? start) (unchecked-min (dec len) start)
                    (neg? start) (+ len start)
                    :else start)]
         (if (>= idx 0)
@@ -1695,7 +1695,7 @@ reduces them without incurring seq initialization"
 
   ICounted
   (-count [_]
-    (max 0 (- (alength arr) i)))
+    (unchecked-max 0 (- (alength arr) i)))
 
   IIndexed
   (-nth [coll n]
@@ -2801,17 +2801,32 @@ reduces them without incurring seq initialization"
    :added "1.11.10"}
   [a] (Math/abs a))
 
+(defn NaN?
+  "Returns true if num is NaN, else false"
+  [val]
+  (js/isNaN val))
+
 (defn ^number max
   "Returns the greatest of the nums."
   ([x] x)
-  ([x y] (cljs.core/max x y))
+  ([x y]
+   (cond
+     (NaN? x) x
+     (NaN? y) y
+     (> x y) x
+     :else y))
   ([x y & more]
    (reduce max (cljs.core/max x y) more)))
 
 (defn ^number min
   "Returns the least of the nums."
   ([x] x)
-  ([x y] (cljs.core/min x y))
+  ([x y]
+   (cond
+     (NaN? x) x
+     (NaN? y) y
+     (< x y) x
+     :else y))
   ([x y & more]
    (reduce min (cljs.core/min x y) more)))
 
@@ -6156,7 +6171,7 @@ reduces them without incurring seq initialization"
     (let [v-pos (+ start n)]
       (if (or (neg? n) (<= (inc end) v-pos))
         (throw (js/Error. (str_ "Index " n " out of bounds [0," (-count coll) "]")))
-        (build-subvec meta (assoc v v-pos val) start (max end (inc v-pos)) nil))))
+        (build-subvec meta (assoc v v-pos val) start (unchecked-max end (inc v-pos)) nil))))
 
   IReduce
   (-reduce [coll f]
@@ -9924,7 +9939,7 @@ reduces them without incurring seq initialization"
 
   IChunkedSeq
   (-chunked-first [rng]
-    (IntegerRangeChunk. start step (min cnt 32)))
+    (IntegerRangeChunk. start step (unchecked-min cnt 32)))
   (-chunked-rest [rng]
     (if (<= cnt 32)
       ()
@@ -10326,7 +10341,7 @@ reduces them without incurring seq initialization"
       (cons match-vals
             (lazy-seq
              (let [post-idx (+ (.-index matches)
-                               (max 1 (.-length match-str)))]
+                               (unchecked-max 1 (.-length match-str)))]
                (when (<= post-idx (.-length s))
                  (re-seq* re (subs s post-idx)))))))))
 
@@ -12163,11 +12178,6 @@ reduces them without incurring seq initialization"
   [x]
   (instance? goog.Uri x))
 
-(defn NaN?
-  "Returns true if num is NaN, else false"
-  [val]
-  (js/isNaN val))
-
 (defn ^:private parsing-err
   "Construct message for parsing for non-string parsing error"
   [val]
@@ -12296,7 +12306,7 @@ reduces them without incurring seq initialization"
         -1
         (loop [idx (cond
                      (pos? start) start
-                     (neg? start) (max 0 (+ start len))
+                     (neg? start) (unchecked-max 0 (+ start len))
                      :else start)]
           (if (< idx len)
             (if (= (-nth coll idx) x)
@@ -12309,7 +12319,7 @@ reduces them without incurring seq initialization"
       (if (zero? len)
         -1
         (loop [idx (cond
-                     (pos? start) (min (dec len) start)
+                     (pos? start) (unchecked-min (dec len) start)
                      (neg? start) (+ len start)
                      :else start)]
           (if (>= idx 0)

src/main/clojure/cljs/core.cljc

@@ -1201,17 +1201,21 @@
 (core/defmacro ^::ana/numeric neg? [x]
   `(< ~x 0))
 
-(core/defmacro ^::ana/numeric max
+(core/defmacro ^::ana/numeric unchecked-max
   ([x] x)
-  ([x y] `(let [x# ~x, y# ~y]
-            (~'js* "((~{} > ~{}) ? ~{} : ~{})" x# y# x# y#)))
-  ([x y & more] `(max (max ~x ~y) ~@more)))
+  ([x y]
+   `(let [x# ~x, y# ~y]
+      (if (> x# y#) x# y#)))
+  ([x y & more]
+   `(max (max ~x ~y) ~@more)))
 
-(core/defmacro ^::ana/numeric min
+(core/defmacro ^::ana/numeric unchecked-min
   ([x] x)
-  ([x y] `(let [x# ~x, y# ~y]
-            (~'js* "((~{} < ~{}) ? ~{} : ~{})" x# y# x# y#)))
-  ([x y & more] `(min (min ~x ~y) ~@more)))
+  ([x y]
+   `(let [x# ~x, y# ~y]
+      (if (< x# y#) x# y#)))
+  ([x y & more]
+   `(min (min ~x ~y) ~@more)))
 
 (core/defmacro ^::ana/numeric js-mod [num div]
   (core/list 'js* "(~{} % ~{})" num div))

src/test/cljs/cljs/core_test.cljs

@@ -1986,3 +1986,10 @@
                  (str x obj y "\"foobar\"" 1 :foo nil))]
     (testing "object is stringified using toString"
       (is (= "correct6\"foobar\"1:foo" (str-fn nil (+ 1 2 3)))))))
+
+(deftest test-cljs-3425
+  (testing "Incorrect min/max handling of ##NaN"
+    (is (NaN? (min ##NaN 1)))
+    (is (NaN? (min 1 ##NaN)))
+    (is (NaN? (max ##NaN 1)))
+    (is (NaN? (max 1 ##NaN)))))