Summary
Syntax-rules expansion uses unprefixed identifiers which can be captured by module imports
Metadata
- Id: fcfcb81590a2593cde446b2a608be2c99f0f661d
- Trac id: 511
- Type: defect
- Reporter: sjamaan
- Owner: felix
- Cc:
- Status: closed
- Component: expander
- Estimated difficulty:
- Resolution: worksforme
- Priority: critical
- Milestone: 4.9.0
- Version: 4.6.x
- Changetime: 2012-09-24 21:47:48 UTC
- Created: 2011-02-19 13:34:02 UTC
- Keywords: declare, macro-expanded code
Description
It appears that "declare" statements in macro expanded code don't work properly anymore: http://tests.call-cc.org/2011/02/19/salmonella-report/snowtar.html
Changes and comments
[2011-02-19 14:04:56 UTC] sjamaan changed priority from major to critical
[2011-02-19 17:21:54 UTC] sjamaan changed status from new to assigned
[2011-02-19 17:21:54 UTC] sjamaan changed owner from sjamaan to felix
[2011-02-19 17:21:54 UTC] sjamaan wrote:
Turns out this has nothing to do with "declare" after all, but it was a bug in my patch for #379 (forgot to rename "-")
It turns out that even renaming it properly does not help - because the library imports an global identifier called "-", that will always be used. If this is a bug, we should really look at that! I suppose it may not really be a bug as syntax-rules is a macro-generating macro; it would have to have some way to refer to the definition of - at the definition site of syntax-rules, not at the definition site of the macro that it generates (I hope this makes sense... *My* head almost exploded...).
Another example is that I was unable to rename "length" in such a way that it couldn't be captured like this:
(module capture (length)
(import chicken (only scheme define car))
(define (length x)
(car x)))
(module foo (bar)
(import chicken scheme capture)
(define-syntax def
(syntax-rules ()
((_ name value ...)
(define (name) (list value ...)))))
(def bar 1))
I had to work around that by adding a "safe" ##sys#length to library.scm. The minus I simply changed to plus, and added -1 to the value. Alternatively, I could've done the same as for length, to add ##sys#- to library.scm.
These immediate bugs in syntax-rules have been fixed in changeset 0eb8724 of the expander-simplifications branch; I've also removed a couple of unused local definitions there.
We should check that the non-##sys#-prefixed identifiers like the special forms and and such -- but also the free identifiers like compare and such! --
[2011-02-19 17:22:44 UTC] sjamaan changed summary
[2011-02-19 17:27:56 UTC] sjamaan wrote:
Replying to sjamaan: > We should check that the non-##sys#-prefixed identifiers like the special forms and and such -- but also the free identifiers like compare and such! --
I got cut off there. I meant to say we should check that all non-prefixed identifiers are safe and cannot be captured in whatever way.
[2011-02-19 17:42:38 UTC] sjamaan wrote:
I was a bit surprised that snowtar complains that numbers#- is not defined though. It's good that it does that, and I added a regression test that makes this behaviour explicitly defined: hashed prefixed references to other modules' identifiers should not work in modules.
See expander-simplifications changeset 0862e159 for this regression test. I hope you agree this is a good idea.
[2011-02-19 19:59:00 UTC] sjamaan wrote:
I tried hard but couldn't break it. Possibly the problem does not occur for macros or free identifiers. Should we try to fix it for bound identifiers?
[2011-02-20 15:33:00 UTC] felix wrote:
Sorry, Peter, but I don't understand a single word.
[2011-02-20 15:39:51 UTC] felix wrote:
Replying to felix: > Sorry, Peter, but I don't understand a single word.
Ok, I tried to install snowtar - are you saying that the expansion of a `syntax-rules` contains expander-specific code that captures identifiers? Is this the problem?
[2011-02-20 15:41:29 UTC] sjamaan wrote:
syntax-rules is a macro. Its expansion includes renamed identifiers for "length", among others.
But this renaming doesn't appear to work properly; if there are imported procedures with the same name, those are used instead of the ones from library.scm. So yes, I think the problem is "that the expansion of a syntax-rules contains expander-specific code that captures identifiers".
I think this has something to do with the fact that it's a macro-generating macro. I will try to come up with a simpler testcase.
[2011-02-20 15:42:27 UTC] sjamaan wrote:
My changes in git get rid of this problem by using only ##sys#-prefixed names in its expansion.
[2011-02-22 22:03:29 UTC] sjamaan wrote:
I had another look at the generated code and it turns out I was wrong: the length procedure is only used in the generated macro's code, not in the expansion of that macro. This makes it even more peculiar that it's not working!
It's as if the "rename" procecudure isn't working properly with imports. However, I'm not able to reproduce this. Possible it only happens with the "core" syntax env.
[2011-02-22 22:05:48 UTC] sjamaan wrote:
I mean I'm not able to reproduce this in a simple testcase!
I can easily reproduce it with the above code snippet in the second comment here, after I change this line in synrules.scm:
(define %length '##sys#length)
to
(define %length (r 'length)
[2011-03-19 12:13:13 UTC] sjamaan changed status from assigned to closed
[2011-03-19 12:13:13 UTC] sjamaan set resolution to worksforme
[2011-03-19 12:13:13 UTC] sjamaan wrote:
Just ignore my silly ramblings
[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