Skip to content

Commit

Permalink
CLJS-2476: recur across try should fail compilation
Browse files Browse the repository at this point in the history 
Additionally fixes a few places where
recur was incorrectly used in cljs.js.
  • Loading branch information
@mfikes @swannodette
mfikes authored and swannodette committed Feb 11, 2018
1 parent af20aea commit a1d49b5
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 16 deletions.
39 changes: 26 additions & 13 deletions src/main/cljs/cljs/js.cljs
View file
Expand Up @@ -656,14 +656,23 @@
(when (and (seq deps) emit-nil-result?)
(.append sb "null;"))))

(defn- trampoline-safe
"Returns a new function that calls f but discards any return value,
returning nil instead, thus avoiding any inadvertent trampoline continuation
if a function happens to be returned."
[f]
(comp (constantly nil) f))

(defn- analyze-str* [bound-vars source name opts cb]
(let [rdr (rt/indexing-push-back-reader source 1 name)
cb (trampoline-safe cb)
eof (js-obj)
aenv (ana/empty-env)
the-ns (or (:ns opts) 'cljs.user)
bound-vars (cond-> (merge bound-vars {:*cljs-ns* the-ns})
(:source-map opts) (assoc :*sm-data* (sm-data)))]
((fn analyze-loop [last-ast ns]
(trampoline
(fn analyze-loop [last-ast ns]
(binding [env/*compiler* (:*compiler* bound-vars)
ana/*cljs-ns* ns
ana/*checked-arrays* (:checked-arrays opts)
Expand Down Expand Up @@ -699,12 +708,12 @@
(cb res)
(let [ast (:value res)]
(if (#{:ns :ns*} (:op ast))
(ns-side-effects bound-vars aenv ast opts
((trampoline-safe ns-side-effects) bound-vars aenv ast opts
(fn [res]
(if (:error res)
(cb res)
(analyze-loop ast (:name ast)))))
(recur ast ns)))))
(trampoline analyze-loop ast (:name ast)))))
#(analyze-loop ast ns)))))
(cb {:value last-ast}))))))) nil the-ns)))

(defn analyze-str
Expand Down Expand Up @@ -883,13 +892,15 @@

(defn- compile-str* [bound-vars source name opts cb]
(let [rdr (rt/indexing-push-back-reader source 1 name)
cb (trampoline-safe cb)
eof (js-obj)
aenv (ana/empty-env)
sb (StringBuffer.)
the-ns (or (:ns opts) 'cljs.user)
bound-vars (cond-> (merge bound-vars {:*cljs-ns* the-ns})
(:source-map opts) (assoc :*sm-data* (sm-data)))]
((fn compile-loop [ns]
(trampoline
(fn compile-loop [ns]
(binding [env/*compiler* (:*compiler* bound-vars)
*eval-fn* (:*eval-fn* bound-vars)
ana/*cljs-ns* ns
Expand Down Expand Up @@ -928,18 +939,18 @@
[node-libs (assoc ast :deps libs-to-load)])
[nil ast])]
(if (#{:ns :ns*} (:op ast))
(ns-side-effects bound-vars aenv ast opts
((trampoline-safe ns-side-effects) bound-vars aenv ast opts
(fn [res]
(if (:error res)
(cb res)
(let [ns-name (:name ast)]
(.append sb (with-out-str (comp/emit (:value res))))
(when-not (nil? node-deps)
(node-side-effects bound-vars sb node-deps ns-name (:def-emits-var opts)))
(compile-loop (:name ast))))))
(trampoline compile-loop (:name ast))))))
(do
(.append sb (with-out-str (comp/emit ast)))
(recur ns))))))
#(compile-loop ns))))))
(do
(when (:source-map opts)
(append-source-map env/*compiler*
Expand Down Expand Up @@ -1013,6 +1024,7 @@

(defn- eval-str* [bound-vars source name opts cb]
(let [rdr (rt/indexing-push-back-reader source 1 name)
cb (trampoline-safe cb)
eof (js-obj)
aenv (ana/empty-env)
sb (StringBuffer.)
Expand All @@ -1021,7 +1033,8 @@
(:source-map opts) (assoc :*sm-data* (sm-data)))
aname (cond-> name (:macros-ns opts) ana/macro-ns-name)]
(when (:verbose opts) (debug-prn "Evaluating" name))
((fn compile-loop [ns]
(trampoline
(fn compile-loop [ns]
(binding [env/*compiler* (:*compiler* bound-vars)
*eval-fn* (:*eval-fn* bound-vars)
ana/*cljs-ns* ns
Expand Down Expand Up @@ -1065,7 +1078,7 @@
(do
(.append sb
(with-out-str (comp/emitln (str "goog.provide(\"" (comp/munge (:name ast)) "\");"))))
(ns-side-effects true bound-vars aenv ast opts
((trampoline-safe ns-side-effects) true bound-vars aenv ast opts
(fn [res]
(if (:error res)
(cb res)
Expand All @@ -1076,11 +1089,11 @@
(filter ana/dep-has-global-exports? (:deps ast))
ns-name
(:def-emits-var opts))
(compile-loop ns'))))))
(trampoline compile-loop ns'))))))
(do
(env/with-compiler-env (assoc @(:*compiler* bound-vars) :options opts)
(.append sb (with-out-str (comp/emit ast))))
(recur ns'))))))
#(compile-loop ns'))))))
(do
(when (:source-map opts)
(append-source-map env/*compiler*
Expand All @@ -1105,7 +1118,7 @@
(wrap-error (ana/error aenv "ERROR" cause))))]
(cb res)))))]
(if-let [f (:cache-source opts)]
(f evalm complete)
((trampoline-safe f) evalm complete)
(complete {:value nil}))))))))))
(:*cljs-ns* bound-vars))))

Expand Down
6 changes: 3 additions & 3 deletions src/main/clojure/cljs/analyzer.cljc
View file
Expand Up @@ -1421,7 +1421,7 @@
parser))

finally (when (seq fblock)
(analyze (assoc env :context :statement) `(do ~@(rest fblock))))
(disallowing-recur (analyze (assoc env :context :statement) `(do ~@(rest fblock)))))
e (when (or (seq cblocks) dblock) (gensym "e"))
default (if-let [[_ _ name & cb] dblock]
`(cljs.core/let [~name ~e] ~@cb)
Expand All @@ -1444,8 +1444,8 @@
:column (get-col e env)})
locals)
catch (when cblock
(analyze (assoc catchenv :locals locals) cblock))
try (analyze (if (or e finally) catchenv env) `(do ~@body))]
(disallowing-recur (analyze (assoc catchenv :locals locals) cblock)))
try (disallowing-recur (analyze (if (or e finally) catchenv env) `(do ~@body)))]

{:env env :op :try :form form
:try try
Expand Down
9 changes: 9 additions & 0 deletions src/test/clojure/cljs/analyzer_tests.clj
View file
Expand Up @@ -820,6 +820,15 @@
(a/analyze-file (io/file "src/test/cljs_build/analyzer_test/no_defs.cljs"))))
(is (= {} (get-in @test-cenv [::a/namespaces 'analyzer-test.no-defs :defs]))))

(deftest test-cljs-2476
(doseq [invalid-try-recur-form '[(loop [] (try (recur)))
(loop [] (try (catch js/Error t (recur))))
(loop [] (try (catch :default t (recur))))
(loop [] (try (finally (recur))))]]
(is (thrown-with-msg? Exception
#"Can't recur here"
(a/analyze test-env invalid-try-recur-form)))))

(comment
(binding [a/*cljs-ns* a/*cljs-ns*]
(a/no-warn
Expand Down

0 comments on commit a1d49b5

Please sign in to comment.