basilisp-lang / basilisp

A Clojure-compatible(-ish) Lisp dialect targeting Python 3.8+
https://basilisp.readthedocs.io
Eclipse Public License 1.0
229 stars 5 forks source link

Functions created in loop macro don't properly capture environment. #990

Open mitch-kyle opened 2 weeks ago

mitch-kyle commented 2 weeks ago

I'm getting a RecursionError trying to build functions with loop. It looks like the bindings from subsequent iterations are leaking into created functions.

This form shows the bug.

user> 
  (loop [f    #(prn 1)
         rec? true]
    (if rec?
      (recur (fn [] (f) (prn 2))
             false)
      (do (prn 0)
          (f))))

Expected Result:

0
1
2

Actual Result:

0
Traceback (most recent call last):
  File "/home/mitch/Sources/basilisp/src/basilisp/contrib/nrepl_server.lpy", line 136, in do_handle_eval
    (let [result (last
                 ^^^^^
  File "/home/mitch/Sources/basilisp/src/basilisp/lang/runtime.py", line 1831, in trampoline
    ret = f(*args, **kwargs)
          ^^^^^^^^^^^^^^^^^^
  File "/home/mitch/Sources/basilisp/src/basilisp/core.lpy", line 480, in last
    (if (seq (rest s))
             ^^^^^^^^
  File "/usr/lib/python3.12/functools.py", line 909, in wrapper
    return dispatch(args[0].__class__)(*args, **kw)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mitch/Sources/basilisp/src/basilisp/lang/runtime.py", line 1070, in rest
    n = to_seq(o)
        ^^^^^^^^^
  File "/usr/lib/python3.12/functools.py", line 909, in wrapper
    return dispatch(args[0].__class__)(*args, **kw)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mitch/Sources/basilisp/src/basilisp/lang/seq.py", line 274, in _to_seq_lazyseq
    return o.seq()
           ^^^^^^^
  File "/home/mitch/Sources/basilisp/src/basilisp/lang/seq.py", line 174, in seq
    self._compute_seq()
  File "/home/mitch/Sources/basilisp/src/basilisp/lang/seq.py", line 169, in _compute_seq
    self._obj = gen()
                ^^^^^
  File "/home/mitch/Sources/basilisp/src/basilisp/contrib/nrepl_server.lpy", line 138, in ____do_handle_eval__for_5846_5875__lisp_fn_5877
    (basilisp.lang.compiler/compile-and-exec-form form
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mitch/Sources/basilisp/src/basilisp/lang/compiler/__init__.py", line 192, in compile_and_exec_form
    exec(bytecode, ns.module.__dict__)  # pylint: disable=exec-used
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "*cider-repl Sources/basilisp:localhost:54427(clj)*", line 7, in <module>
  File "*cider-repl Sources/basilisp:localhost:54427(clj)*", line 5, in __lisp_fn_899
  File "*cider-repl Sources/basilisp:localhost:54427(clj)*", line 5, in __lisp_fn_899
  File "*cider-repl Sources/basilisp:localhost:54427(clj)*", line 5, in __lisp_fn_899
  [Previous line repeated 966 more times]
RecursionError: maximum recursion depth exceeded
mitch-kyle commented 2 weeks ago

are there any performance benefits to using loop over fn? because if not, a quick fix for this could be to define loop like:

(defmacro loop
  [bindings & body]
  `((fn ~(vec (take-nth 2 bindings)) 
      ~@body)
     ~@(take-nth 2 (rest bindings))))