Handle shadowing of bindings through macroexpansion

[czan]

I've added two test cases to show the type of expansion that breaks without this change. The crux of it is that we need to provide an environment to macroexpand, but it doesn't let us do so. To get it to work we cheat a bit and invoke the macro function itself in approximately the same way as Compiler.java does.

[mszajna]

Big thank you for your contribution! I'm reviewing your changes right now and I'll try to push out an updated version as soon as I can.

I would like to make sure I understand the underlying issue though and see if there's anything about the generative testing I could improve not to miss such cases in the future.

[czan]

The issue is when var resolution (with resolve) doesn't take into account locally established bindings. This isn't a problem for run-time values, because the rewritten code will maintain the visible bindings and everything will work out. Clojure allows macros to be shadowed, though, so that means that during macroexpansion the local bindings (ie. &env) need to be taken into account, too. Due to the way async recursively macroexpands we need to augment &env to also include the other binding forms that we see along the way.

In terms of adding it to the code generation, I think it would suffice to have a macro defined, and then generate calls to the macro and bindings of the same name. Something like:

;; define the macro at the top level
(defmacro test-macro [x] `(inc ~x))

;; then teach the generators to generate forms like:
`(~'test-macro ~a-number) ;; we have to be careful with the namespace of the symbol
`(let [test-macro (fn [x#] (- x#))] ~another-generated-form)

[mszajna]

Thanks for clarifying. I'm going to give code gen a longer think - it's likely to be somewhat tricky with the interplay of namespace mappings and macroexpansion as per the comment on shadowing-symbols test.

Another 2 binding constructs, loop and catch, are likely affected by shadowing issues as well. Would you be happy to cover them too or rather have me pick it up from there?

I'm otherwise satisfied with this PR and unless you want to update it further I'm inclined to merge it in. Thanks again for your contribution and thanks for engaging with this library in the first place! I would love to hear about your use-case, if you don't mind of course.

[czan]

I'd like to fix up loop and catch before merging. I think the namespace mapping issue can be solved by binding *ns* in the test (like I suggested for the shadowing one above), so I would also love to get the code gen part in if we can. I'll see if I can get something working.

[czan]

Okay, I've added support for loop* and catch, and fixed the test, but I took a quick look at the generators and it's going to be more involved than I thought, so I'm not going to do that right now. I'm happy for you to merge this PR whenever you want.