Skip to content

Commit

Permalink
CLJS-3309 (#96)
Browse files Browse the repository at this point in the history
The `and/or` optimization removes the intermediate let, however the child expressions will still have the local in their environments, and more importantly anon fns in loops will have captured the local in the `:loop-lets` AST field. We add a pass that removes the local as well as loop-lets. Add compile and runtime tests based on cases from the ticket.
  • Loading branch information
swannodette authored May 19, 2021
1 parent 8ef4bd2 commit 15f330f
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 17 deletions.
32 changes: 32 additions & 0 deletions src/main/cljs/cljs/analyzer/passes.cljc
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
;; Copyright (c) Rich Hickey. All rights reserved.
;; The use and distribution terms for this software are covered by the
;; Eclipse Public License 1.0 (http://opensource.org/licenses/eclipse-1.0.php)
;; which can be found in the file epl-v10.html at the root of this distribution.
;; By using this software in any fashion, you are agreeing to be bound by
;; the terms of this license.
;; You must not remove this notice, or any other, from this software.

(ns cljs.analyzer.passes)

(defn apply-passes
([ast passes]
(apply-passes ast passes nil))
([ast passes opts]
(reduce
(fn [ast pass]
(pass (:env ast) ast opts))
ast passes)))

(defn walk
([ast passes]
(walk ast passes nil))
([ast passes opts]
(reduce
(fn [ast child-k]
(assoc ast
child-k
(let [child (get ast child-k)]
(if (vector? child)
(into [] (map #(walk % passes opts)) child)
(walk child passes)))))
(some-> ast (apply-passes passes opts)) (:children ast))))
55 changes: 38 additions & 17 deletions src/main/cljs/cljs/analyzer/passes/and_or.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
;; the terms of this license.
;; You must not remove this notice, or any other, from this software.

(ns cljs.analyzer.passes.and-or)
(ns cljs.analyzer.passes.and-or
(:require [cljs.analyzer.passes :as passes]))

(def simple-ops
#{:var :js-var :local :invoke :const :host-field :host-call :js :quote})
Expand Down Expand Up @@ -70,25 +71,45 @@
(and (simple-or? ast)
(simple-test-expr? (-> ast :body :ret :else))))

(defn remove-loop-let [fn-ast local]
(update fn-ast :loop-lets
(fn [loop-lets]
(map
(fn [m]
(update m :params
(fn [xs] (remove #(= local (:name %)) xs))))
loop-lets))))

(defn remove-local-pass [local]
(fn [env ast opts]
(cond-> (update-in ast [:env :locals] dissoc local)
(= :fn (:op ast)) (remove-loop-let local))))

(defn optimize-and [ast]
{:op :js
:env (:env ast)
:segs ["((" ") && (" "))"]
:args [(-> ast :bindings first :init)
(->expr-env (-> ast :body :ret :then))]
:form (:form ast)
:children [:args]
:tag 'boolean})
(let [{:keys [init name]} (-> ast :bindings first)]
{:op :js
:env (:env ast)
:segs ["((" ") && (" "))"]
:args [init
(passes/walk
(->expr-env (-> ast :body :ret :then))
[(remove-local-pass name)])]
:form (:form ast)
:children [:args]
:tag 'boolean}))

(defn optimize-or [ast]
{:op :js
:env (:env ast)
:segs ["((" ") || (" "))"]
:args [(-> ast :bindings first :init)
(->expr-env (-> ast :body :ret :else))]
:form (:form ast)
:children [:args]
:tag 'boolean})
(let [{:keys [init name]} (-> ast :bindings first)]
{:op :js
:env (:env ast)
:segs ["((" ") || (" "))"]
:args [init
(passes/walk
(->expr-env (-> ast :body :ret :else))
[(remove-local-pass name)])]
:form (:form ast)
:children [:args]
:tag 'boolean}))

(defn optimize [env ast _]
(cond
Expand Down
12 changes: 12 additions & 0 deletions src/test/cljs/cljs/core_test.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -1854,3 +1854,15 @@
(is (false? (contains? sv :kw))))
(let [sv (subvec [0 1 2 3 4] 2 2)]
(is (false? (contains? sv 0)))))

(deftest test-cljs-3309
(is (= :ok
(loop [x 4]
(if (or (< x 4) (not-any? (fn [y] x) [1]))
(recur 5)
:ok))))
(is (= '([])
((fn [s]
(for [e s :when (and (sequential? e) (every? (fn [x] x) e))]
e))
[[]]))))
48 changes: 48 additions & 0 deletions src/test/clojure/cljs/analyzer_pass_tests.clj
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

(ns cljs.analyzer-pass-tests
(:require [cljs.analyzer :as ana]
[cljs.analyzer.passes :as passes]
[cljs.analyzer.passes.and-or :as and-or]
[cljs.analyzer-tests :as ana-tests :refer [analyze]]
[cljs.compiler :as comp]
Expand All @@ -16,6 +17,33 @@
[clojure.string :as string]
[clojure.test :as test :refer [deftest is testing]]))

(deftest test-walk
(testing "walking visits every node"
(let [expr-env (assoc (ana/empty-env) :context :expr)
ast (->> `(and true false)
(analyze expr-env))
ast' (passes/walk ast [(fn [_ ast _] (dissoc ast :env))])]
(is (not (contains? ast' :env)))
(is (not (some #(contains? % :env) (:args ast')))))))

(deftest remove-local
(testing "and/or remove local pass"
(let [ast {:op :fn
:env '{:locals {x {}}}
:loop-lets '[{:params [{:name x}]}]}
pass (and-or/remove-local-pass 'x)
ast' (passes/apply-passes ast [pass])]
(is (contains? (-> ast :env :locals) 'x))
(is (not (contains? (-> ast' :env :locals) 'x)))
(is (some
(fn [{:keys [params]}]
(some #(= 'x (:name %)) params))
(:loop-lets ast)))
(is (not (some
(fn [{:keys [params]}]
(some #(= 'x (:name %)) params))
(:loop-lets ast')))))))

(deftest test-and-or-code-gen-pass
(testing "and/or optimization code gen pass"
(let [expr-env (assoc (ana/empty-env) :context :expr)
Expand Down Expand Up @@ -110,6 +138,26 @@
(and (even? 1) false))]))))]
(is (= 1 (count (re-seq #"&&" code)))))))

(deftest test-cljs-3309
(testing "CLJS-3309: and/or optimization removes discarded local and loop-lets"
(let [code (env/with-compiler-env (env/default-compiler-env)
(comp/with-core-cljs {}
(fn []
(compile-form-seq
'[(loop [x 4]
(when (or (< x 4) (not-any? (fn [y] x) [1]))
(recur 5)))]))))]
(is (empty? (re-seq #"or_" code))))
(let [code (env/with-compiler-env (env/default-compiler-env)
(comp/with-core-cljs {}
(fn []
(compile-form-seq
'[((fn [s]
(for [e s :when (and (sequential? e) (every? (fn [x] x) e))]
e))
[[]])]))))]
(is (empty? (re-seq #"and_" code))))))

(comment
(test/run-tests)

Expand Down

0 comments on commit 15f330f

Please sign in to comment.