Summary

define-syntax in begin-for-syntax body ineffective

Metadata

Attachments

Description

`(begin-for-syntax (define-syntax ...))` doesn't seem to work. I assume this is caused by the use of `parameterize` in `eval/meta` (eval.scm): restoring `##sys#macro-environment` drops any new syntax definitions made.

Changes and comments

[2011-02-24 20:48:17 UTC] sjamaan wrote:

Shouldn't macros be evaluated with the meta-macro-environment as their macro environment? (ie, use eval/meta while expanding macro definitions)

[2011-02-24 20:53:37 UTC] sjamaan wrote:

Never mind the idiot; that's exactly what it's being used for

[2011-02-24 21:37:42 UTC] sjamaan wrote:

OK, I got it to work in the interpreter. Here's a simple testcase:

 (module foo (listify)
   (import chicken scheme)
   (begin-for-syntax
    (define-syntax call-it-123
      (syntax-rules ()
        ((_ x)
         '(x 'x 1 2 3)))))
   (define-syntax listify
     (lambda (e r c)
       (call-it-123 list))))
 
 (import foo)
 (print (listify))

The patch works because while expanding the body of a macro, the macro environment should be that of the meta-macro-environment, but inside the thunk that is the second argument to dynamic-wind the macro-environment has already been tweaked to point to what was previously the meta-macro-environment. (at least, I think that's what's happening)

I can't get it to work in the compiler for some reason. I've replaced the compiler's eval/meta to have the same code as in eval.scm (which is more careful in restoring the various environments and works differently to a regular parameterize), but that didn't help :(

[2011-02-24 21:37:57 UTC] sjamaan attached meta-macros.diff (description=Fix for the interpreter)

[2011-02-24 21:38:43 UTC] sjamaan wrote:

The example fragment is expected to print (list 1 2 3) by the way

[2011-02-24 22:02:43 UTC] sjamaan wrote:

Replying to sjamaan: > I can't get it to work in the compiler for some reason.

Actually, strike that. I got it to work in the compiler too, but I needed to install it to see the effects. Stupid testcases :( (actually, I needed to recompile chicken with itself after doing this, because it segfaulted initially. There may be a bug in there somewhere, unless this is expected)

I've committed and pushed a fix to the expander-simplifications branch (revision 216f2181)

[2011-02-25 08:57:19 UTC] felix wrote:

I can't follow. I already made the change. Do you refer to my changed code? I thought this was fixed already. Our coordination sucks.

[2011-02-25 09:41:14 UTC] sjamaan wrote:

Replying to felix: > I can't follow. I already made the change. Do you refer to my changed code? I thought this was fixed already.

I wasn't aware you had fixed it, and unaware of that I was actually working on your fixed code. The fix didn't work for me. Also, you didn't patch the compiler; just the interpreter. My changeset adds a test, too. Please give it a try.

> Our coordination sucks.

It would be a good idea if you put a comment on a ticket or close it at the same time you're pushing a fix.

[2011-03-05 15:18:09 UTC] sjamaan wrote:

This works now

[2011-03-05 15:18:09 UTC] sjamaan changed status from new to closed

[2011-03-05 15:18:09 UTC] sjamaan set resolution to fixed

[2011-06-01 09:00:41 UTC] felix changed milestone from 4.7.0 to 4.8.0

[2011-06-01 09:00:41 UTC] felix wrote:

Milestone 4.7.0 deleted

[2012-09-24 21:47:48 UTC] felix changed milestone from 4.8.0 to 4.9.0

[2012-09-24 21:47:48 UTC] felix wrote:

Milestone 4.8.0 deleted