From 2f8866d825331e901d17335b915e9771787ac344 Mon Sep 17 00:00:00 2001 From: David Nolen Date: Mon, 17 May 2021 19:58:49 -0400 Subject: [PATCH 1/5] `or` failing example --- src/test/clojure/cljs/analyzer_pass_tests.clj | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/clojure/cljs/analyzer_pass_tests.clj b/src/test/clojure/cljs/analyzer_pass_tests.clj index 76521e7df..610e5fc29 100644 --- a/src/test/clojure/cljs/analyzer_pass_tests.clj +++ b/src/test/clojure/cljs/analyzer_pass_tests.clj @@ -115,4 +115,13 @@ (require '[clojure.pprint :refer [pprint]]) + (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)))]))))] + code) + ) From f21f242311d12a163103283c0a7af4e9dd13838f Mon Sep 17 00:00:00 2001 From: David Nolen Date: Tue, 18 May 2021 10:06:56 -0400 Subject: [PATCH 2/5] add walk, add test --- src/main/cljs/cljs/analyzer/passes.cljc | 32 +++++++++++++++++++ src/test/clojure/cljs/analyzer_pass_tests.clj | 10 ++++++ 2 files changed, 42 insertions(+) create mode 100644 src/main/cljs/cljs/analyzer/passes.cljc diff --git a/src/main/cljs/cljs/analyzer/passes.cljc b/src/main/cljs/cljs/analyzer/passes.cljc new file mode 100644 index 000000000..5d1a3a9cf --- /dev/null +++ b/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)))) diff --git a/src/test/clojure/cljs/analyzer_pass_tests.clj b/src/test/clojure/cljs/analyzer_pass_tests.clj index 610e5fc29..fb141ff6b 100644 --- a/src/test/clojure/cljs/analyzer_pass_tests.clj +++ b/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,15 @@ [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 test-and-or-code-gen-pass (testing "and/or optimization code gen pass" (let [expr-env (assoc (ana/empty-env) :context :expr) From 29795af40d461184ea7219bac381bee3ced983fb Mon Sep 17 00:00:00 2001 From: David Nolen Date: Wed, 19 May 2021 11:04:49 -0400 Subject: [PATCH 3/5] add pass to remove a local from envs and loop-lets, add test case for new pass --- .../cljs/cljs/analyzer/passes/and_or.cljc | 55 +++++++++++++------ src/test/clojure/cljs/analyzer_pass_tests.clj | 18 ++++++ 2 files changed, 56 insertions(+), 17 deletions(-) diff --git a/src/main/cljs/cljs/analyzer/passes/and_or.cljc b/src/main/cljs/cljs/analyzer/passes/and_or.cljc index c4fe8e5be..52bc76c8a 100644 --- a/src/main/cljs/cljs/analyzer/passes/and_or.cljc +++ b/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 diff --git a/src/test/clojure/cljs/analyzer_pass_tests.clj b/src/test/clojure/cljs/analyzer_pass_tests.clj index fb141ff6b..9cdaa43f6 100644 --- a/src/test/clojure/cljs/analyzer_pass_tests.clj +++ b/src/test/clojure/cljs/analyzer_pass_tests.clj @@ -26,6 +26,24 @@ (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) From 89b2530a71ea71deb80f42a14ef959e4c90bd247 Mon Sep 17 00:00:00 2001 From: David Nolen Date: Wed, 19 May 2021 11:09:33 -0400 Subject: [PATCH 4/5] add CLJS-3309 test cases demonstrating fix --- src/test/clojure/cljs/analyzer_pass_tests.clj | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/test/clojure/cljs/analyzer_pass_tests.clj b/src/test/clojure/cljs/analyzer_pass_tests.clj index 9cdaa43f6..a9cc90166 100644 --- a/src/test/clojure/cljs/analyzer_pass_tests.clj +++ b/src/test/clojure/cljs/analyzer_pass_tests.clj @@ -138,18 +138,29 @@ (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) (require '[clojure.pprint :refer [pprint]]) - (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)))]))))] - code) - ) From c2be16721b66541e7c32a7caff393f3be9a53fbf Mon Sep 17 00:00:00 2001 From: David Nolen Date: Wed, 19 May 2021 11:14:57 -0400 Subject: [PATCH 5/5] add runtime tests for CLJS-3309 --- src/test/cljs/cljs/core_test.cljs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/cljs/cljs/core_test.cljs b/src/test/cljs/cljs/core_test.cljs index 65ffcc631..8c3ad289c 100644 --- a/src/test/cljs/cljs/core_test.cljs +++ b/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)) + [[]]))))