Fork me on GitHub

cljs.analyzer/numeric-type? is still a bit relaxed here and I wonder if the correct test might be (contains? '#{js js/Number} t) This change would at least catch (< (js/Date.) (js/Date.)), with a

WARNING: cljs.core/<, all arguments must be numbers, got [js/Date js/Date] instead
diagnostic, but I wonder if it is too restrictive. The js is needed for forms like (< (js/parseInt "3") (js/parseInt "4")).


@dnolen High level summary of all the map entry tickets: With the latest patch in each (which is currently CLJS-2456.patch, CLJS-2477-3.patch, CLJS-2482.patch, CLJS-2460-2.patch, and CLJS-2478-2.patch), all tests pass, including the 30,977 tests / 121,688 assertions currently in Coal Mine. A potentially consequential change in CLJS-2478-2.patch is that vectors are no longer map entries. (Code that does something like (key [:a 1]) was always incorrect, but with the change it would actually fail.)


@mfikes Does (into {} [[[:a 2] [:b 3]]]) also pass? It looks like it might fail because the loop still uses key/val


Also, what's perf hit on this? The satisfies? is more expensive than implements and when -conj!ing vectors it will always be false for the first map-entry? check.


@rauh I haven't done any perf testing on the set of changes. Here is the result of your into question:

cljs.user=> (into {} [[[:a 2] [:b 3]]])
{[:a 2] [:b 3]}


@mfikes Oh sorry, I mean to do: (into {} (list [:a 2] [:b 3]))


This happens to work in CLJS currently, since it fails the vector & map-entry? check, and then creates a {:a 2, :b 3} map. Though it doesn't work in CLJ. So...


that also works in clj


@bronsa Right. 3rd try is the charm. What I'm trying to say: This currently happens to work in CLJS (but not in CLJ):

(-> (transient {})
    (conj! (seq [[:a 2] [:b 3]]))


minimal repro is (conj {} '([1 2]))



(-> (transient {})
    (conj! (seq [[:a 2] [:b 3]]))
fails with the changes with
Error: No protocol method IMapEntry.-key defined for type cljs.core/PersistentVector: [:a 2]


Thanks to the recent -Sdeps enhancement to the clojure tool, you can run Coal Mine against a ClojureScript master commit with a one-liner, without manually cloning anything:

clojure -Sdeps '{:deps {coal-mine {:git/url "" :sha "8486879d5381aae2eaa81f1404e152ff53afaab6"} org.clojure/clojurescript {:git/url "" :sha "9ddd356d344aa1ebf9bd9443dd36a1911c92d32f"} {:mvn/version "v20170910"}}}' -J-Xmx3G -m coal-mine.script test
This ends up looking like
Checking out:  at 8486879d5381aae2eaa81f1404e152ff53afaab6
Checking out:  at 9ddd356d344aa1ebf9bd9443dd36a1911c92d32f

Building coal-mine.test-runner-1 ...
Running coal-mine.test-runner-1 in Node ...

Testing coal-mine.problem-1

Testing coal-mine.problem-2


Of course, you can also run it against a local checkout of the ClojureScript tree by using :local/root instead. I’m liking this flexibility 🙂

Alex Miller (Clojure team)22:01:50

good to see all the pieces coming together