CLJS-3309 (#96)

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.

Author: swannodette

src/main/cljs/cljs/analyzer/passes.cljc

@@ -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))))

src/main/cljs/cljs/analyzer/passes/and_or.cljc

@@ -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})
@@ -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

src/test/cljs/cljs/core_test.cljs

@@ -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))
+          [[]]))))

src/test/clojure/cljs/analyzer_pass_tests.clj

@@ -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]
@@ -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)
@@ -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)