Skip to content

Commit

Permalink
CLJS-700: Fix for clojure.core.reducers/fold. Previously (r/fold + + …
Browse files Browse the repository at this point in the history
…[1 2 3]) returned the wrong result.
  • Loading branch information
jdevuyst authored and swannodette committed Nov 29, 2013
1 parent f02775e commit f768cd5
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 20 deletions.
49 changes: 30 additions & 19 deletions src/cljs/clojure/core/reducers.cljs
Expand Up @@ -14,8 +14,21 @@
:author "Rich Hickey"}
clojure.core.reducers
(:refer-clojure :exclude [reduce map mapcat filter remove take take-while drop flatten])
(:require [clojure.walk :as walk]
[cljs.core :as core]))
(:require [cljs.core :as core]))

;;;;;;;;;;;;;; some fj stuff ;;;;;;;;;;
(defn- fjtask [f]
f)

(defn- fjinvoke [f]
(f))

(defn- fjfork [task]
task)

(defn- fjjoin [task]
(task))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(defn reduce
"Like core/reduce except:
Expand All @@ -30,12 +43,9 @@
(array? coll) (array-reduce coll f init)
:else (-reduce coll f init)))))

#_
(defprotocol CollFold
(coll-fold [coll n combinef reducef]))

;;; TODO: update docstring for CLJS
#_
(defn fold
"Reduces a collection using a (potentially parallel) reduce-combine
strategy. The collection is partitioned into groups of approximately
Expand All @@ -45,14 +55,14 @@
reducef). combinef must be associative, and, when called with no
arguments, (combinef) must produce its identity element. These
operations may be performed in parallel, but the results will
preserve order."
preserve order.
Note: Performing operations in parallel is currently not implemented."
([reducef coll] (fold reducef reducef coll))
([combinef reducef coll] (fold 512 combinef reducef coll))
([n combinef reducef coll]
(coll-fold coll n combinef reducef)))

(def fold reduce)

(defn reducer
"Given a reducible collection, and a transformation function xf,
returns a reducible collection, where any supplied reducing
Expand All @@ -79,9 +89,7 @@
(-reduce [_ f1 init]
(-reduce coll (xf f1) init))

#_
CollFold
#_
(coll-fold [_ n combinef reducef]
(coll-fold coll n combinef (xf reducef))))))

Expand Down Expand Up @@ -188,7 +196,7 @@

cljs.core/ISeqable
(-seq [_] (concat (seq left) (seq right)))

cljs.core/IReduce
(-reduce [this f1] (-reduce this f1 (f1)))
(-reduce
Expand All @@ -197,9 +205,7 @@
right f1
(-reduce left f1 init)))

#_
CollFold
#_
(coll-fold
[this n combinef reducef]
(-reduce this reducef)))
Expand Down Expand Up @@ -263,7 +269,6 @@
(into [] (r/flatten nil))
)

(comment
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; fold impls ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(defn- foldvec
[v n combinef reducef]
Expand All @@ -282,20 +287,26 @@
(combinef (f1) (fjjoin t2)))))))

(extend-protocol CollFold
Object
nil
(coll-fold
[coll n combinef reducef]
(combinef))

object
(coll-fold
[coll n combinef reducef]
;;can't fold, single reduce
(reduce reducef (combinef) coll))
clojure.lang.IPersistentVector

cljs.core/IPersistentVector
(coll-fold
[v n combinef reducef]
(foldvec v n combinef reducef))

clojure.lang.PersistentHashMap
#_
cljs.core/PersistentHashMap
#_
(coll-fold
[m n combinef reducef]
(.fold m n combinef reducef fjinvoke fjtask fjfork fjjoin)))

)
18 changes: 17 additions & 1 deletion test/cljs/cljs/reducers_test.cljs
Expand Up @@ -8,7 +8,23 @@
(assert (= 10 (r/reduce + (array 1 2 3 4))))
(assert (= 11 (r/reduce + 1 (array 1 2 3 4))))
(assert (= 10 (r/reduce + (list 1 2 3 4))))
(assert (= 11 (r/reduce + 1 (list 1 2 3 4)))))
(assert (= 11 (r/reduce + 1 (list 1 2 3 4))))
(assert (= (r/fold + + [1 2 3])
(r/fold + [1 2 3])
(r/reduce + [1 2 3])
6))
(assert (= (r/fold + + (vec (range 2048)))
(r/reduce + (vec (range 2048)))))
(letfn [(f [[ks vs] k v]
[(conj ks k) (conj vs v)])
(g ([] [#{} #{}])
([[ks1 vs1] [ks2 vs2]]
[(into ks1 ks2) (into vs1 vs2)]))]
(assert (= (r/reduce f (g) {:a 1 :b 2 :c 3})
(r/fold g f {:a 1 :b 2 :c 3})
[#{:a :b :c} #{1 2 3}]))
(let [m (into {} (for [x (range 2048)] [x (- x)]))]
(assert (= (r/reduce f (g) m) (r/fold g f m))))))

(defn test-all []
(test-builtin-impls))

0 comments on commit f768cd5

Please sign in to comment.