Instrumenting hash-map gives StackOverflow on JVM

[borkdude]

The cause is probably that spec-checking-fn calls with-instrument-disabled, which calls binding which expands in a call to hash-map, so there's a loop. See https://github.com/clojure/clojure/blob/ee3553362de9bc3bfd18d4b0b3381e3483c2a34c/src/clj/clojure/core.clj#L1947.

Repro:

$ clj -A:test -m cljs.main -re node

ClojureScript 1.10.439
cljs.user=> (require '[speculative.core])
nil
cljs.user=> (require '[clojure.spec.test.alpha :as stest])
nil
cljs.user=> (stest/instrument `hash-map)
[cljs.core/hash-map]
cljs.user=> (apply hash-map [1])
repl:13
throw e__6573__auto__;
^

Error: Call to #'cljs.core/hash-map did not conform to spec.

$ clj -A:test

Clojure 1.10.0
user=> (require '[speculative.core])
nil
user=> (require '[clojure.spec.test.alpha :as stest])
nil
user=> (stest/instrument `hash-map)
[clojure.core/hash-map]
user=> (apply hash-map [1])
Execution error (StackOverflowError) at (REPL:1).
null

A possible solution would be to replace in the binding macro: (push-thread-bindings (hash-map ~@(var-ize bindings))) with (push-thread-bindings (clojure.lang.PersistentHashMap/create ~(var-ize bindings))), i.e. inline the hash-map call.

An additional improvement would be to make the call to hash-map at macro-expansion time:

(defmacro binding
  "binding => var-symbol init-expr

  Creates new bindings for the (already-existing) vars, with the
  supplied initial values, executes the exprs in an implicit do, then
  re-establishes the bindings that existed before.  The new bindings
  are made in parallel (unlike let); all init-exprs are evaluated
  before the vars are bound to their new values."
  {:added "1.0"}
  [bindings & body]
  (assert-args
    (vector? bindings) "a vector for its binding"
    (even? (count bindings)) "an even number of forms in binding vector")
  (let [var-ize (fn [var-vals]
                  (loop [ret [] vvs (seq var-vals)]
                    (if vvs
                      (recur  (conj (conj ret `(var ~(first vvs))) (second vvs))
                             (next (next vvs)))
                      (apply hash-map (seq ret)))))]
    `(let []
       (push-thread-bindings ~(var-ize bindings))
       (try
         ~@body
         (finally
           (pop-thread-bindings))))))

Benchmark:

user=> (time (dotimes [_ 10000000] (binding [*print-length* 10])))
"Elapsed time: 3754.649969 msecs"
nil
user=> (time (dotimes [_ 10000000] (binding* [*print-length* 10])))
"Elapsed time: 1356.306343 msecs"

With this fix (hash-map 1 1) works, but (hash-map 1) doesn't yet.