fixed issue with variadic ampersand symbol in macros

[ikappaki]

HI,

could you please review patch to treat the ampersand operator in macros as a standalone symbol. Fixes #772.

Test included.

thanks

[chrisrink10]

I acknowledge that this seems like a Basilisp bug, but this fix is concerning to me. It feels like it's in far too general of a placement to fix the bug it's fixing.

While I contemplate this patch, I believe you can do this to get around the issue:

(defmacro ^:private variadic-fn []
  `(fn [~'& r#] r#))

[ikappaki]

I acknowledge that this seems like a Basilisp bug, but this fix is concerning to me. It feels like it's in far too general of a placement to fix the bug it's fixing.

While I contemplate this patch, I believe you can do this to get around the issue:

(defmacro ^:private variadic-fn []
  `(fn [~'& r#] r#))

Hi, I understand from the above that you do acknowledge the incompatibility with Clojure, and, maybe, that the current behavior is somehow confusing (very confusing to me in the case of defining macros that define fns).

I had a closer looks and a found a more targeted place to apply the patch with the latest commit, in the syntax quote logic.

This brings it closer to the Clojure behaviour (which covers the macro case), with one exception that explicitly namespace qualified ampersand is also treated as a quoted ampersand

basilisp

basilisp.user=> `&
&

basilisp.user=> `a
basilisp.user/a

basilisp.user=> `user/&
&

clojure

user> `&
&
user> `a
user/a
user> `user/&
user/&

It is not obvious to me how can I treat this case in the syntax-quote code, since the namespace is added by the reader if missing, and there is no indication whether it was specified by the user or the reader.

what do you think?

Thanks

[chrisrink10]

I think your previous fix is probably fine after reviewing it again. I can see the problem with the latest patch and I don't see any easy way around it.

[ikappaki]

I think your previous fix is probably fine after reviewing it again. I can see the problem with the latest patch and I don't see any easy way around it.

I've reverted to the previous patch with some minor optimisation while adding the `xxx/& test case.