Opened 11 years ago

Closed 11 years ago

Last modified 9 years ago

#511 closed defect (worksforme)

Syntax-rules expansion uses unprefixed identifiers which can be captured by module imports

Reported by: sjamaan Owned by: felix winkelmann
Priority: critical Milestone: 4.9.0
Component: expander Version: 4.6.x
Keywords: declare, macro-expanded code Cc:
Estimated difficulty:

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

Change History (15)

comment:1 Changed 11 years ago by sjamaan

Priority: majorcritical

comment:2 Changed 11 years ago by sjamaan

Owner: changed from sjamaan to felix winkelmann
Status: newassigned

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

comment:3 Changed 11 years ago by sjamaan

Summary: Expander problem with "declare" triggered by snowtarSyntax-rules expansion uses unprefixed identifiers which can be captured by module imports

comment:4 in reply to:  2 Changed 11 years ago by sjamaan

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.

comment:5 Changed 11 years ago by sjamaan

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.

comment:6 Changed 11 years ago by sjamaan

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?

comment:7 Changed 11 years ago by felix winkelmann

Sorry, Peter, but I don't understand a single word.

comment:8 in reply to:  7 Changed 11 years ago by felix winkelmann

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?

comment:9 Changed 11 years ago by sjamaan

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.

comment:10 Changed 11 years ago by sjamaan

My changes in git get rid of this problem by using only ##sys#-prefixed names in its expansion.

comment:11 Changed 11 years ago by sjamaan

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.

comment:12 Changed 11 years ago by sjamaan

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)

comment:13 Changed 11 years ago by sjamaan

Resolution: worksforme
Status: assignedclosed

Just ignore my silly ramblings

comment:14 Changed 11 years ago by felix winkelmann

Milestone: 4.7.04.8.0

Milestone 4.7.0 deleted

comment:15 Changed 9 years ago by felix winkelmann

Milestone: 4.8.04.9.0

Milestone 4.8.0 deleted

Note: See TracTickets for help on using tickets.