CHICKEN Scheme

Summary

Nested let-syntax failure to strip renamed identifiers

Metadata

Id: ba8ba6efb37116ce53b9832d3211ca12bfe51c64
Trac id: 398
Type: defect
Reporter: sjamaan
Owner: felix
Cc:
Status: closed
Component: expander
Estimated difficulty:
Resolution: fixed
Priority: major
Milestone: 4.9.0
Version: 4.6.x
Changetime: 2012-09-24 21:47:48 UTC
Created: 2010-09-22 21:26:01 UTC
Keywords: expander, renamed identifiers

Attachments

ba8ba6efb37116ce53b9832d3211ca12bfe51c64/attachments/syntax-tests.patch

Description

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.

Changes and comments

[2010-09-22 21:28:52 UTC] sjamaan wrote:

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

[2010-09-22 21:30:14 UTC] sjamaan removed owner

[2010-09-22 21:30:14 UTC] sjamaan changed component from unknown to expander

[2010-09-26 13:57:34 UTC] sjamaan wrote:

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)))))

[2010-09-26 14:01:06 UTC] sjamaan wrote:

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.

[2010-09-26 14:10:16 UTC] sjamaan wrote:

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.

[2010-09-26 15:50:48 UTC] sjamaan wrote:

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

[2010-09-26 16:19:30 UTC] sjamaan wrote:

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.

[2010-09-27 09:33:01 UTC] sjamaan wrote:

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.

[2010-09-28 14:48:52 UTC] felix changed priority from minor to major

[2010-09-28 14:48:52 UTC] felix wrote:

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.

[2010-09-28 20:04:10 UTC] sjamaan wrote:

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.

[2010-09-28 20:04:49 UTC] sjamaan attached syntax-tests.patch (description=syntax-rules, er-macro and ir-macro versions of Al*'s evil testcase)

[2010-09-28 20:28:50 UTC] sjamaan wrote:

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.

[2010-09-29 08:30:49 UTC] felix changed status from new to assigned

[2010-09-29 08:30:49 UTC] felix set owner to felix

[2010-09-29 08:30:49 UTC] felix wrote:

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.