Nested let-syntax failure to strip renamed identifiers

[sjamaan]

I dug up the old

   (let ((a 1))
     (letrec-syntax
       ((foo (syntax-rules ()
               ((_ b)
                (bar a b))))  
        (bar (syntax-rules ()
               ((_ c d)    
                (cons c (let ((c 3))
                          (list d c 'c)))))))
        (let ((a 2))  
          (foo a))))

This returns "(1 2 3 a0)" in Chicken. I think there's a missing call to ##sys#strip-syntax somewhere. I haven't looked into the expander code yet, but I wanted to post this so I don't forget.

[sjamaan]

That first line should say:

I dug up the old ​An Advanced Syntax-Rules Primer for the Mildly Insane by Al* Petrofsky, and found an example on which Chicken fails to expand correctly.

I should get some sleep

[sjamaan]

Here's what I believe to be the equivalent ER macro:

(let ((a 1))
  (letrec-syntax
      ((foo (er-macro-transformer
             (lambda (x r c)
               `(,(r 'bar) ,(r 'a) ,(cadr x)))))
       (bar (er-macro-transformer
             (lambda (x r c)
               (let ((c (cadr x))
                     (d (caddr x)))
                `(,(r 'cons) ,c
                  (,(r 'let) ((,c 3))
                   (,(r 'list) ,d ,c ',c))))))))
    (let ((a 2))
      (t '(1 2 3 a) (foo a)))))

[sjamaan]

Just for kicks, the corresponding ir-macro:

(let ((a 1))
  (letrec-syntax
      ((foo (ir-macro-transformer
             (lambda (x i c)
               `(bar a ,(cadr x)))))
       (bar (ir-macro-transformer
             (lambda (x i c)
               (let ((c (cadr x))
                     (d (caddr x)))
                `(cons ,c
                       (let ((,c 3))
                         (list ,d ,c ',c))))))))
    (let ((a 2))
      (foo a))))

All three macros fail, so it's not likely that the bug is in the syntax-rules expander. It's more likely that quote doesn't strip enough syntax. Will investigate further.

[sjamaan]

Interestingly, that ER macro returns (1 3 3 a) in MIT-scheme. So I must have made a mistake in the transcription to ER macros or something else is up.

[sjamaan]

Tested in chibi-scheme. Same result: (1 3 3 a)

[sjamaan]

The correct er-macro:

(let ((a 1))
  (letrec-syntax
      ((foo (er-macro-transformer
             (lambda (x r c)
               `(,(r 'bar) ,(r 'a) ,(cadr x)))))
       (bar (er-macro-transformer
             (lambda (x r c)
               (let ((c (cadr x))
                     (d (caddr x)))
                `(,(r 'cons) ,(cadr x)
                  (,(r 'let) ((,c 3))
                   (,(r 'list) ,d ,c ',c))))))))
    (let ((a 2))
      (foo a))))

This returns (1 2 3 a186) (still wrong, of course)

I have a feeling that the ir-macro returning the wrong result (1 1 3 a85) is due to the same error. Not 100% sure yet.

[sjamaan]

I prepared a simplified example of what's really happening here:

(let ((a 1))
  (let-syntax ((foo (syntax-rules ()
                      ((_) (quote a)))))
    (foo)))

ER version:

(let ((a 1))
  (let-syntax ((foo (lambda (x r c)
                     `(quote ,(r 'a)))))
    (foo)))

Both these examples result in a renamed symbol showing up in the output.

[felix winkelmann]

This patch seems to help (at least runs the test-suite). It is incomplete (must be done for all binding forms) and should be refactored (say, a system-function in expand.scm that extends an SE with a given set of aliased variables), but may indicate a solution: the SE of foo contains (a . a0) mapping a to a gensym. At the point where quote (really: ##core#quote) strips syntax off the argument form, the mapping of a to a0 is not available anymore (it exists only inside the static SE of foo). Here I register the original name on the plist of the generated gensym which will be seen by strip-syntax. Looks somewhat hackish, but seems to work.

diff --git a/compiler.scm b/compiler.scm
index 5ae4664..e198367 100644
--- a/compiler.scm
+++ b/compiler.scm
@@ -612,6 +612,9 @@
                                (aliases (map gensym vars))
                                (se2 (append (map cons vars aliases) se)) )
                           (set-real-names! aliases vars)
+                          (for-each
+                           (cut ##sys#put! <> '##core#real-name <>)
+                           aliases vars)
                           `(let
                             ,(map (lambda (alias b)
                                     (list alias (walk (cadr b) e se (car b) #t)) )
diff --git a/eval.scm b/eval.scm
index b531547..12f927b 100644
--- a/eval.scm
+++ b/eval.scm
@@ -393,6 +393,9 @@
                                 (aliases (map gensym vars))
                                 [e2 (cons aliases e)]
                                 (se2 (append (map cons vars aliases) se))
+                                (_ (for-each
+                                    (cut ##sys#put! <> '##core#real-name <>)
+                                    aliases vars))
                                 [body (##sys#compile-to-closure
                                        (##sys#canonicalize-body (cddr x) se2 #f)
                                        e2

Please have a look. If this works, I can try to integrate this in a more cleaner way.

[sjamaan]

Fantastic! It works now, and I'm very relieved to see that my gut feeling was correct: the IR macro problem also went away ;)

Now all three examples return (1 2 3 a)!

Here's a patch that adds these bastards to the testsuite.

[sjamaan]

syntax-rules, er-macro and ir-macro versions of Al*'s evil testcase

[sjamaan]

Why not add this property setting code to gensym? It seems like a useful thing to keep around on all generated symbols, even outside the context of Chicken core.

Here's a silly idea: I don't know how and where gensym is usually used, but I can imagine it might even make sense to have a procedure (original-symbol-name <sym>) which retrieves this property, if available, to "undo" gensym calls. If not available, it would just return its argument unmodified.

[felix winkelmann]

Replying to sjamaan:

Why not add this property setting code to gensym? It seems like a useful thing to keep around on all generated symbols, even outside the context of Chicken core.

I don't know. Gensym is used very often, and I'm not sure such a feature would be required very often (the idea has a certain logic to it, yet I can't think of much use that would justify the extra code and memory requirements).

Here's a silly idea: I don't know how and where gensym is usually used, but I can imagine it might even make sense to have a procedure (original-symbol-name <sym>) which retrieves this property, if available, to "undo" gensym calls. If not available, it would just return its argument unmodified.

Hm.

[felix winkelmann]

Milestone 4.7.0 deleted

[felix winkelmann]

Milestone 4.8.0 deleted