error macroexpanding recur macro

[ikappaki]

Basilisp throws an error when trying to macroexpand a well formulated macro with a recur fn:
basilisp.lang.compiler.exception.CompilerException: no recur point defined for recur.

To reproduce

1. Open up a REPL and create a simple loop/recur macro

  basilisp.user=> (defmacro ab [& body]
                    `(loop [x# true]
                       (if x#
                         (do ~@body)
                         :done)))
  #'basilisp.user/ab

2. Try to macroexpand the macro, an error is thrown

basilisp.user=> (macroexpand-1 '(ab (recur false)))
Traceback (most recent call last):
  File "C:\src\basilisp\src\basilisp\cli.py", line 446, in repl
    result = eval_str(lsrc, ctx, ns, eof)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
  File "C:\src\basilisp\src\basilisp\lang\compiler\analyzer.py", line 3256, in _list_node
    return handle_special_form(form, ctx)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\src\basilisp\src\basilisp\lang\compiler\analyzer.py", line 2877, in _recur_ast
    raise ctx.AnalyzerException("no recur point defined for recur", form=form)
basilisp.lang.compiler.exception.CompilerException: no recur point defined for recur {:form (recur false) :phase :analyzing :file "<Macroexpand>"}

Calling the macro works fine otherwise

basilisp.user=> (ab (recur false))
:done

It also works fine in Clojure

user> (defmacro ab [& body]
                    `(loop [x# true]
                       (if x#
                         (do ~@body)
                         :done)))
#'user/ab
user> (macroexpand-1 '(ab (recur false)))
(clojure.core/loop
 [x__8388__auto__ true]
 (if x__8388__auto__ (do (recur false)) :done))

Thanks

[ikappaki]

Hi @chrisrink10,

Do you have any suggestions on how to address this problem?

adding and not ctx.should_allow_unresolved_symbols: in the below resolves this specific issue, but fails a compiler test. Nonetheless, it seems somewhat crude to force this

@_analyze_form.register(llist.PersistentList)
@_analyze_form.register(ISeq)
@_with_loc
def _list_node(form: ISeq, ctx: AnalyzerContext) -> Node:
    if form == llist.PersistentList.empty():
        with ctx.quoted():
            return _const_node(form, ctx)

    if ctx.is_quoted:
        return _const_node(form, ctx)

    s = form.first
    if isinstance(s, sym.Symbol):
        handle_special_form = _SPECIAL_FORM_HANDLERS.get(s)
        if handle_special_form is not None and not ctx.should_allow_unresolved_symbols:
            return handle_special_form(form, ctx)
        elif s.name.startswith(".-"):
            return _host_prop_ast(form, ctx)
        elif s.name.startswith(".") and s.name != _DOUBLE_DOT_MACRO_NAME:
            return _host_call_ast(form, ctx)

    return _invoke_ast(form, ctx)

thanks

[chrisrink10]

I think I have an idea for a solution, but I'm not really clear on why this is something anyone would do. recur should almost certainly just be part of a macro's definition, not an argument.

[ikappaki]

I think I have an idea for a solution, but I'm not really clear on why this is something anyone would do. recur should almost certainly just be part of a macro's definition, not an argument.

This reproducible test case is an over simplification of an issue I've encountered while porting the clojure.pprint library. It occurs in particular when Basilisp analyzes the pprintsimple-list fn:

https://github.com/clojure/clojure/blob/e3520c07e8b21dbf5a91fa14b7114b90dc1e89c6/src/clj/clojure/pprint/dispatch.clj#L76-L85

(defn- pprint-simple-list [alis]
  (pprint-meta alis)
  (pprint-logical-block :prefix "(" :suffix ")"
    (print-length-loop [alis (seq alis)]
      (when alis
	(write-out (first alis))
	(when (next alis)
	  (.write ^java.io.Writer *out* " ")
	  (pprint-newline :linear)
	  (recur (next alis)))))))

The exception on the REPL is

basilisp.user=> (require '[basilisp.pprint :as pp])

  exception: <class 'basilisp.lang.compiler.exception.CompilerException'> from <class 'basilisp.lang.compiler.exception.CompilerException'>
      phase: :macroexpansion
    message: error occurred during macroexpansion: no recur point defined for recur
       form: (when (next alis) (.write *out* " ") (pprint-newline :linear) (recur (next alis)))
   location: <Macroexpand>:83-89

Btw, the new exception format looks fantastic, though for some reason we are missing the source file path in the output making it very difficult to locate the code location where the error is coming from.

[chrisrink10]

Btw, the new exception format looks fantastic, though for some reason we are missing the source file path in the output making it very difficult to locate the code location where the error is coming from.

Are you actually using macroexpand or macroexpand-1 here? The <Macroexpand> location is used only when invoked via one of those two functions. Actual macroexpansion will use the filename passed to the compiler.

Is your basilisp.pprint implementation on Github yet so I can inspect the file?

[ikappaki]

Btw, the new exception format looks fantastic, though for some reason we are missing the source file path in the output making it very difficult to locate the code location where the error is coming from.

Are you actually using macroexpand or macroexpand-1 here? The <Macroexpand> location is used only when invoked via one of those two functions. Actual macroexpansion will use the filename passed to the compiler.

Is your basilisp.pprint implementation on Github yet so I can inspect the file?

There is no macroexpansion used here (as in me not calling macroexpand*), it is just basilisp trying to load the above code.

The code is not on GH yet, I'll see if I can contrive an example that it fails as such by just trying to load the fn.

[ikappaki]

Hi,

I found the culprit, it is indeed macroexpand that is eventually called after all in the pprint code via pprint-simple-list as you rightly suggested

https://github.com/clojure/clojure/blob/e3520c07e8b21dbf5a91fa14b7114b90dc1e89c6/src/clj/clojure/pprint/pprint_base.clj#L380-L389

(defn- pll-mod-body [var-sym body]
  (letfn [(inner [form]
                 (if (seq? form)
                   (let [form (macroexpand form)] 
                     (condp = (first form)
                       'loop* form
                       'recur (concat `(recur (inc ~var-sym)) (rest form))
                       (walk inner identity form)))
                   form))]
    (walk inner identity body)))

and the error can be reproduced as

basilisp.user=> (require '[basilisp.walk :refer [walk]])
nil

basilisp.user=> (defn- pll-mod-body [var-sym body]
                  (letfn [(inner [form]
                                 (if (seq? form)
                                   (let [form (macroexpand form)]
                                     (condp = (first form)
                                       'loop* form
                                       'recur (concat `(recur (inc ~var-sym)) (rest form))
                                       (walk inner identity form)))
                                   form))]
                    (walk inner identity body)))
#'basilisp.user/pll-mod-body

basilisp.user=> (pll-mod-body nil '(when alis
                                     (write-out (first alis))
                                     (when (next alis)
                                       ;; (prlabel --pprint-simple-list (next alis))
                                       (.write ^Writer *out* " ")
                                       ;; (prlabel --pprint-simple-list :1)
                                       (pprint-newline :linear)
                                       ;; (prlabel --pprint-simple-list :2)
                                       (recur (next alis)))))

  exception: <class 'basilisp.lang.compiler.exception.CompilerException'> from <class 'basilisp.lang.compiler.exception.CompilerException'>
      phase: :macroexpansion
    message: error occurred during macroexpansion: no recur point defined for recur
       form: (when (next alis) (.write *out* " ") (pprint-newline :linear) (recur (next alis)))
   location: <Macroexpand>:NO_SOURCE_LINE

contrast with Clojure which has no such constraint

user> (require '[clojure.walk :as walk])
nil
user> (defn- pll-mod-body [var-sym body]
                  (letfn [(inner [form]
                                 (if (seq? form)
                                   (let [form (macroexpand form)]
                                     (condp = (first form)
                                       'loop* form
                                       'recur (concat `(recur (inc ~var-sym)) (rest form))
                                       (walk inner identity form)))
                                   form))]
                    (walk inner identity body)))
#'user/pll-mod-body
user> (pll-mod-body nil '(when alis
                                     (write-out (first alis))
                                     (when (next alis)
                                       ;; (prlabel --pprint-simple-list (next alis))
                                       (.write ^Writer *out* " ")
                                       ;; (prlabel --pprint-simple-list :1)
                                       (pprint-newline :linear)
                                       ;; (prlabel --pprint-simple-list :2)
                                       (recur (next alis)))))
(when
 alis
 (write-out (first alis))
 (if
  (next alis)
  (do
   (. *out* write " ")
   (pprint-newline :linear)
   (recur (clojure.core/inc nil) (next alis)))))

My humble opinion is that Basilisp should not be restricting users how to organise their code, this seems like a valid case to me.

Thanks