Skip to content

Commit 1525386

Browse files
borkdudeswannodette
authored andcommitted
CLJS-2995: Instrumented self-calling multi-arity fn throws maximum call stack exceeded with optimizations advanced
Fixed by calling static arities instead of apply
1 parent 0be62b2 commit 1525386

File tree

3 files changed

+43
-16
lines changed

3 files changed

+43
-16
lines changed

src/main/cljs/cljs/spec/test/alpha.cljc

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -288,16 +288,22 @@ spec itself will have an ::s/failure value in ex-data:
288288
(fn [sym]
289289
(do `(check-1 '~sym nil nil ~opts-sym)))))]))))
290290

291-
(defmacro ^:private maybe-setup-static-dispatch [f ret arity]
291+
(defmacro ^:private maybe-setup-static-dispatch [f ret conform! arity]
292292
(let [arity-accessor (symbol (str ".-cljs$core$IFn$_invoke$arity$" arity))
293293
argv (mapv #(symbol (str "arg" %)) (range arity))]
294-
`(when (some? (~arity-accessor ~f))
294+
`(when-some [ac# (~arity-accessor ~f)]
295295
(set! (~arity-accessor ~ret)
296-
(fn ~argv
297-
(apply ~ret ~argv))))))
298-
299-
(defmacro ^:private setup-static-dispatches [f ret max-arity]
296+
(fn ~argv
297+
(if *instrument-enabled*
298+
(with-instrument-disabled
299+
(~conform! ~argv)
300+
(binding [*instrument-enabled* true]
301+
(ac# ~@argv)))
302+
(ac# ~@argv)))))))
303+
304+
(defmacro ^:private setup-static-dispatches [f ret conform! max-arity]
305+
;; ret is for when we don't have arity info
300306
`(do
301307
~@(mapv (fn [arity]
302-
`(maybe-setup-static-dispatch ~f ~ret ~arity))
308+
`(maybe-setup-static-dispatch ~f ~ret ~conform! ~arity))
303309
(range (inc max-arity)))))

src/main/cljs/cljs/spec/test/alpha.cljs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@
8787
(defn- spec-checking-fn
8888
[v f fn-spec]
8989
(let [fn-spec (@#'s/maybe-spec fn-spec)
90+
args-spec (:args fn-spec)
9091
conform! (fn [v role spec data args]
9192
(let [conformed (s/conform spec data)]
9293
(if (= ::s/invalid conformed)
@@ -112,15 +113,18 @@
112113
pure-variadic?)
113114
(.cljs$core$IFn$_invoke$arity$variadic f)
114115
(apply f args)))
115-
ret (fn [& args]
116-
(if *instrument-enabled*
117-
(with-instrument-disabled
118-
(when (:args fn-spec) (conform! v :args (:args fn-spec) args args))
119-
(binding [*instrument-enabled* true]
120-
(apply' f args)))
121-
(apply' f args)))]
122-
(when-not pure-variadic?
123-
(setup-static-dispatches f ret 20)
116+
conform!* #(conform! v :args args-spec % %)
117+
ret (if args-spec
118+
(fn [& args]
119+
(if *instrument-enabled*
120+
(with-instrument-disabled
121+
(conform!* args)
122+
(binding [*instrument-enabled* true]
123+
(apply' f args)))
124+
(apply' f args)))
125+
f)]
126+
(when (and (not pure-variadic?) args-spec)
127+
(setup-static-dispatches f ret conform!* 20)
124128
(when-some [variadic (.-cljs$core$IFn$_invoke$arity$variadic f)]
125129
(set! (.-cljs$core$IFn$_invoke$arity$variadic ret)
126130
(fn [& args]

src/test/cljs/cljs/spec/test_test.cljs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,20 @@
117117
(testing "instrument and unstrument return empty coll when no fdef exists"
118118
(is (empty? (stest/instrument `fn-2975)))
119119
(is (empty? (stest/unstrument `fn-2975)))))
120+
121+
(defn fn-2995
122+
([] (fn-2995 0))
123+
([a] (fn-2995 a 1))
124+
([a b] [a b]))
125+
126+
(s/fdef fn-2995
127+
:args (s/cat :a (s/? number?)
128+
:b (s/? number?)))
129+
130+
(deftest test-2995
131+
(stest/instrument `fn-2995)
132+
(testing "instrumented self-calling multi-arity function works"
133+
(is (= [0 1] (fn-2995 0 1)))
134+
(is (= [0 1] (fn-2995 0)))
135+
(is (= [0 1] (fn-2995 0)))
136+
(is (thrown? js/Error (fn-2995 "not a number")))))

0 commit comments

Comments
 (0)