CLJS-3077: Avoid generating unnecessary functions

Only need to capture locals when creating functions inside a
loop or recur'ing fn.

Author: thheller

src/main/clojure/cljs/analyzer.cljc

@@ -2126,12 +2126,15 @@
         type         (::type form-meta)
         proto-impl   (::protocol-impl form-meta)
         proto-inline (::protocol-inline form-meta)
-        menv         (if (> (count meths) 1)
-                       (assoc env :context :expr)
-                       env)
-        menv         (merge menv
-                       {:protocol-impl proto-impl
-                        :protocol-inline proto-inline})
+        menv         (-> env
+                         (cond->
+                           (> (count meths) 1)
+                           (assoc :context :expr))
+                         ;; clear loop flag since method bodies won't be in a loop at first
+                         ;; only tracking this to keep track of locals we need to capture
+                         (dissoc :in-loop)
+                         (merge {:protocol-impl proto-impl
+                                 :protocol-inline proto-inline}))
         methods      (map #(disallowing-ns* (analyze-fn-method menv locals % type (nil? name))) meths)
         mfa          (transduce (map :fixed-arity) max 0 methods)
         variadic     (boolean (some :variadic? methods))
@@ -2166,6 +2169,7 @@
                       :tag 'function
                       :inferred-ret-tag inferred-ret-tag
                       :recur-frames *recur-frames*
+                      :in-loop (:in-loop env)
                       :loop-lets *loop-lets*
                       :jsdoc [js-doc]
                       :max-fixed-arity mfa
@@ -2343,7 +2347,10 @@
                               (partition 2 bindings)
                               widened-tags))
                        bindings)
-        [bes env]    (analyze-let-bindings encl-env bindings op)
+        [bes env]    (-> encl-env
+                         (cond->
+                           (true? is-loop) (assoc :in-loop true))
+                         (analyze-let-bindings bindings op))
         recur-frame  (when (true? is-loop)
                        {:params bes
                         :flag (atom nil)

src/main/clojure/cljs/compiler.cljc

@@ -957,13 +957,17 @@
       (emitln "})()"))))
 
 (defmethod emit* :fn
-  [{variadic :variadic? :keys [name env methods max-fixed-arity recur-frames loop-lets]}]
+  [{variadic :variadic? :keys [name env methods max-fixed-arity recur-frames in-loop loop-lets]}]
   ;;fn statements get erased, serve no purpose and can pollute scope if named
   (when-not (= :statement (:context env))
-    (let [loop-locals (->> (concat (mapcat :params (filter #(and % @(:flag %)) recur-frames))
-                                   (mapcat :params loop-lets))
-                           (map munge)
-                           seq)]
+    (let [recur-params (mapcat :params (filter #(and % @(:flag %)) recur-frames))
+          loop-locals
+          (->> (concat recur-params
+                 ;; need to capture locals only if in recur fn or loop
+                 (when (or in-loop (seq recur-params))
+                   (mapcat :params loop-lets)))
+               (map munge)
+               seq)]
       (when loop-locals
         (when (= :return (:context env))
             (emits "return "))

src/test/clojure/cljs/compiler_tests.clj

@@ -286,6 +286,51 @@
       (is (str/includes? content "cljs.invoke_test.foo_record.foo_field_a;")))))
 #_(test-vars [#'test-optimized-invoke-emit])
 
+(deftest test-cljs-3077
+  (let [opts {}
+        cenv (env/default-compiler-env opts)
+
+        test-compile
+        (fn [code]
+          (env/with-compiler-env cenv
+            (with-out-str
+              (emit
+                (comp/with-core-cljs
+                  opts
+                  (fn [] (analyze aenv code nil opts)))))))
+
+        snippet1
+        (test-compile
+          '(defn wrapper1 [foo]
+             (let [x 1]
+               (prn (fn inner [] foo))
+               (recur (inc foo)))))
+
+        snippet2
+        (test-compile
+          '(defn wrapper2 [foo]
+             (loop [x 1]
+               (prn (fn inner [] x))
+               (recur (inc x))
+               )))
+
+        snippet3
+        (test-compile
+          '(defn no-wrapper1 [foo]
+             (let [x 1]
+               (prn (fn inner [] foo)))))]
+
+    ;; FIXME: not exactly a clean way to test if function wrappers are created or not
+    ;; captures foo,x
+    (is (str/includes? snippet1 "(function (foo,x){"))
+    ;; captures x
+    (is (str/includes? snippet2 "(function (x){"))
+    ;; no capture, no loop or recur
+    (is (not (str/includes? snippet3 "(function (foo,x){")))
+    (is (not (str/includes? snippet3 "(function (foo){")))
+    (is (not (str/includes? snippet3 "(function (x){")))
+    ))
+
 ;; CLJS-1225
 
 (comment