Fork me on GitHub
#cljs-dev
<
2018-11-03
>
borkdude06:11:01

I found one regression with speculative (https://github.com/slipset/speculative) which I can reproduce as follows:

;; run with:

;; clj -Srepro -Sdeps '{:deps {org.clojure/clojurescript {:mvn/version "1.10.439"} org.clojure/test.check {:mvn/version "RELEASE"}}}' -m cljs.main -re node -i callstack.cljs

(ns callstack
  (:require
   [clojure.test.check]
   [clojure.spec.alpha :as s]
   [clojure.spec.test.alpha :as stest]
   [clojure.test :as t]
   ))

(s/fdef clojure.core/=
  :args (s/+ any?)
  :ret boolean?)

(t/deftest =-test
  (stest/instrument)
  (= 1))

(t/run-tests)
(println "done, exiting...")
Error: RangeError: Maximum call stack size exceeded Used to work on 339.

mfikes13:11:03

FWIW, the challenge with CLJS-2956 above is that, inside the instrumented function “wrapper,” it ultimately calls apply on the plain version in order to invoke it, but with instrumentation re-enabled. The intent is to catch anything that the function itself calls. But, the instrumentation is also enabled during the mechanics of apply. So anything instrumented that is called indirectly via apply—in this case, =—can cause infinite recursion.

borkdude13:11:27

@mfikes I had a similar issue here https://github.com/slipset/speculative/blob/master/src/speculative/test.cljc#L81 First I manually unstrumented =, but later I decided to wrap it in a with-instrument-disabled, which did the trick.

borkdude13:11:39

is this because apply uses <=?

borkdude13:11:22

what has changed between 339 and 439 that this doesn’t work anymore?

mfikes13:11:50

@borkdude In this case, apply ultimately ends up calling = here https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/core.cljs#L7067 With 1.10.339 we were using MultiFn to “overwrite various arities, and with 1.10.439, we are overwriting existing arities in a bespoke fashion, and this causes subtle differences in call patterns. I think you can still effectively run into the same sort of issue with 1.10.339, but with slightly different scenarios (if you spec apply itself, for example).

mfikes13:11:41

OK, perhaps apply is not an example that it occurs for.

borkdude13:11:51

ah, but we didn’t enable the test for it, because we ran into https://dev.clojure.org/jira/browse/CLJS-2942

mfikes14:11:15

The fundamental problem, as I see it, is that since the “wrapper” is implemented using ClojureScript, and a lot of the JavaScript code that is emitted in order to support the function dispatch machinery is also implemented by calling on ClojureScript core functions, it becomes meta-circular very quickly when you introduce the concept of specing core functions—and sometimes leads to infinite recursion.

mfikes14:11:13

A corollary: https://dev.clojure.org/jira/browse/CLJS-2956 and similar tickets may only affect projects that attempt to spec core functions. “Regular” projects that spec their own non-core functions may be immune to this class of issues.

borkdude15:11:16

I wonder why we didn’t see this when instrumenting a lot of core functions and then running coal-mine?

thheller16:11:32

looks like there is an externs inference regression in 1.10.439. https://dev.clojure.org/jira/browse/CLJS-2957

richiardiandrea17:11:44

I saw AST changes happened. Are they for the compiler state? I am worried about tooling maintainers (I am the last committer of cljs-tooling). Is there a place where these changes are summarized if breaking?

thheller17:11:51

https://dev.clojure.org/jira/browse/CLJS-1461 depending on how much of the AST you use some of the changes are breaking (renamed :op for example)

richiardiandrea17:11:57

Thank you will check that

richiardiandrea17:11:42

Looks like what I am looking for so thanks for the link once again