clojure.tools.macro/macrolet and let*/loop* protection

[tomoj]

With symbol-macrolet, inner let-bound names are protected from symbol macro expansion:

user> (clojure.tools.macro/symbol-macrolet
       [foo inc]
       (foo 1))
2
user> (clojure.tools.macro/symbol-macrolet
       [foo inc]
       (let [foo identity]
         (foo 1)))
1

But with macrolet, similar protections don't apply:

user> (clojure.tools.macro/macrolet
       [(foo [x] (list `inc x))]
       (foo 1))
2
user> (clojure.tools.macro/macrolet
       [(foo [x] (list `inc x))]
       (let [foo identity]
         (foo 1)))
2

Does this make sense for some reason I don't understand? Is this a bug? I can file an issue with the patch — unless this is really intended behavior?

[Tassilo Horn]

tomoj <t...@tomoj.la> writes:

> With symbol-macrolet, inner let-bound names are protected from symbol
> macro expansion:
>
> user> (clojure.tools.macro/symbol-macrolet
> [foo inc]
> (foo 1))
> 2
> user> (clojure.tools.macro/symbol-macrolet
> [foo inc]
> (let [foo identity]
> (foo 1)))
> 1
>
> But with macrolet, similar protections don't apply:
>
> user> (clojure.tools.macro/macrolet
> [(foo [x] (list `inc x))]
> (foo 1))
> 2
> user> (clojure.tools.macro/macrolet
> [(foo [x] (list `inc x))]
> (let [foo identity]
> (foo 1)))
> 2
>
> Does this make sense for some reason I don't understand?

It's correct. Example 2 works that way, because the symbol macro
expansion creates

(let [inc identity]
(inc 1))

Example 2 works that way, because the normal macro expansion creates

(let [foo identity]
(inc 1))

Since the foo in the let binding vector is not a list, the macro won't
touch it. That's the difference between macros and symbol macros.

Bye,
Tassilo

[Tom Jack]

On Thu, Nov 22, 2012 at 12:04 AM, Tassilo Horn <ts...@gnu.org> wrote:
>
> It's correct. Example 2 works that way, because the symbol macro
> expansion creates
>
> (let [inc identity]
> (inc 1))

Hmm, that's not what mexpand says:
user> (macro/mexpand
'(macro/symbol-macrolet

[foo inc]
(let [foo identity]

(foo 1))))
(do (let* [foo identity] (foo 1)))

[Tassilo Horn]

Tom Jack <t...@tomoj.la> writes:

Hi Tom,

>> It's correct. Example 2 works that way, because the symbol macro
>> expansion creates
>>
>> (let [inc identity]
>> (inc 1))
>
> Hmm, that's not what mexpand says:
> user> (macro/mexpand
> '(macro/symbol-macrolet
> [foo inc]
> (let [foo identity]
> (foo 1))))
> (do (let* [foo identity] (foo 1)))

Yes, I didn't actually expand it practically, only mentally. :-) But the
point is still correct.

Bye,
Tassilo

[Konrad Hinsen]

> Does this make sense for some reason I don't understand? Is this a bug? I
> can file an issue with the patch — unless this is really intended
> behavior?

It's a bug. The behavior of symbol-macrolet is correct; replacing let-bound
symbols would make symbol-macrolet useless in practice. macrolet should
behave like defmacro, which is

user=> (defmacro foo [x] (list `inc x))
#'user/foo
user=> (foo 1)
2
user=> (let [foo identity] (foo 1))
1

Patch welcome!

Konrad.

[Tassilo Horn]

Konrad Hinsen <google...@khinsen.fastmail.net> writes:

>> Does this make sense for some reason I don't understand? Is this a
>> bug? I can file an issue with the patch — unless this is really
>> intended behavior?
>
> It's a bug.

Oh, indeed, I was wrong. I've just validated it against CL.

(macrolet ((foo (x) `(1+ ,x)))
(flet ((foo (x) x))
(foo 1)))
;=> 1

> Patch welcome!

Here it is.

--8<---------------cut here---------------start------------->8---
From 7c481029d94e0af85cf6438d10fa6666f3f2d778 Mon Sep 17 00:00:00 2001
From: Tassilo Horn <ts...@gnu.org>
Date: Mon, 26 Nov 2012 21:01:20 +0100
Subject: [PATCH] Don't apply macro expansion to protected forms

---
src/main/clojure/clojure/tools/macro.clj | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/main/clojure/clojure/tools/macro.clj b/src/main/clojure/clojure/tools/macro.clj
index 33e450d..be1bf43 100644
--- a/src/main/clojure/clojure/tools/macro.clj
+++ b/src/main/clojure/clojure/tools/macro.clj
@@ -69,10 +69,11 @@
(cond
(seq? form)
(let [f (first form)]
- (cond (contains? special-forms f) form
- (contains? macro-fns f) (apply (get macro-fns f) (rest form))
+ (cond (contains? special-forms f) form
+ (and (not (protected? f))
+ (contains? macro-fns f)) (apply (get macro-fns f) (rest form))
(symbol? f) (cond
- (protected? f) form
+ (protected? f) form
; macroexpand-1 fails if f names a class
(class? (ns-resolve *ns* f)) form
:else (let [exp (expand-symbol f)]
--
1.8.0
--8<---------------cut here---------------end--------------->8---

Bye,
Tassilo

[Tom Jack]

I submitted a patch here: http://dev.clojure.org/jira/browse/TMACRO-2

But your patch, Tassilo, not only looks better, it also suffers from
one less bug than my patch — I don't macroexpand-1 if the symbol is
protected. Want to attach your patch to the ticket? And maybe add a
test or two? :)

[Tassilo Horn]

Tom Jack <t...@tomoj.la> writes:

Hi Tom,

Yes, I'll do so. Thanks for the pointer.

Bye,
Tassilo

[Tassilo Horn]

Tassilo Horn <ts...@gnu.org> writes:

Hi again,

>> I submitted a patch here: http://dev.clojure.org/jira/browse/TMACRO-2
>>
>> But your patch, Tassilo, not only looks better, it also suffers from
>> one less bug than my patch — I don't macroexpand-1 if the symbol is
>> protected. Want to attach your patch to the ticket? And maybe add a
>> test or two? :)
>
> Yes, I'll do so. Thanks for the pointer.

Ok, a new patch is attached. While writing test cases, I've noticed
that symbols introduced by `letfn`, expanding to (letfn* [foo (fn* ...)]
...), were also completely missing. So I also added `letfn*` as a form
introducing new protected symbols, too.

Bye,
Tassilo

[Konrad Hinsen]

Tassilo Horn writes:

> Ok, a new patch is attached. While writing test cases, I've noticed
> that symbols introduced by `letfn`, expanding to (letfn* [foo (fn* ...)]
> ...), were also completely missing. So I also added `letfn*` as a form
> introducing new protected symbols, too.

Patch applied. Thanks a lot!

Konrad.

[Tassilo Horn]

Awesome, thanks!

Bye,
Tassilo