Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#444 closed defect (fixed)

Symbol lookup fails in modules in conjunction with renaming macros

Reported by: Moritz Heidkamp Owned by: felix winkelmann
Priority: major Milestone: 4.9.0
Component: expander Version: 4.6.x
Keywords: Cc:
Estimated difficulty:

Description (last modified by Moritz Heidkamp)

I stumbled upon a problem when trying to generate a
foreign-lambda call in an IR macro. Somehow it failed to
re-rename the injected void symbol correctly. This resulted in
it referring to #%void aka ##sys#void. The following
reproduces the problem:

(import foreign)

(foreign-declare "void foo() { printf(\"hi\\n\"); }")

(module foo
  (bar)
  (import chicken scheme foreign)
  
  (define-syntax bar
    (ir-macro-transformer
     (lambda (x i c)
       `((foreign-lambda ,(i 'void) ,(i 'foo)))))))

(import foo)
(bar)

Note that it does work outside the module, though. So does the ER
version:

(er-macro-transformer
     (lambda (x r c)
       `((,(r 'foreign-lambda) void foo))))

This might suggest that the problem lies within the IR macro
implementation but sjamaan is almost certain that the symbol lookup
must be going wrong somewhere and that it only fails in the IR version
because there's more renaming taking place. Thinking about it some
more he concluded that there is a chance that it's a problem in the IR
implementation anyway. This ticket mainly serves as a reminder for him
to think about that some more. However, other interested parties are
welcome to contribute their thoughts as well, of course!

P.S.: Mr Z. found out that (import (except chicken void)) helps
to make the IR version. This might be a work-around for people
experiencing the same issue and looking for a work-around.

Attachments (2)

ir-fix.patch (1.6 KB) - added by sjamaan 10 years ago.
Fix for IR renaming bug
nested-syntax.patch (1.8 KB) - added by sjamaan 10 years ago.
Fix for nested ER macros/syntax-rules as well as IR macros

Download all attachments as: .zip

Change History (17)

comment:1 Changed 10 years ago by sjamaan

After some investigation, I found a difference in behaviour between Chicken and other Schemes:

(define else #f)
(cond (else 1)) => 1

In other Schemes (s48, Mzscheme and Guile - but not Gauche or Chibi) it returns unspecified.

comment:2 Changed 10 years ago by sjamaan

Gambit behaves as Chicken, Gauche and Chibi

comment:3 Changed 10 years ago by sjamaan

eh, the relevance of this comment about "else" is that a COND macro written as IR macro would behave the same as s48 and mzscheme in all cases. One written as ER macro would behave as Chicken (since chicken's cond IS written as an er macro)

comment:4 in reply to:  1 Changed 10 years ago by Moritz Heidkamp

Bigloo and MIT Scheme also behave as Chicken. Ikarus and Ypsilon don't allow definition of else. Racket behaves as s48 and friends.

comment:5 in reply to:  1 Changed 10 years ago by Moritz Heidkamp

And, for the record, Mosh behaves like s48, too :-)

comment:6 Changed 10 years ago by sjamaan

Correction: According to Alex, chibi only behaves like that at the toplevel because this redefines the "else" that "cond" matches against. It returns undefined when this is inside a module.

comment:7 Changed 10 years ago by sjamaan

This very issue (what to do with "free" identifiers) is under consideration for WG1 of R7RS:

http://trac.sacrideo.us/wg/ticket/83

comment:8 Changed 10 years ago by sjamaan

Here's a fun one:

(define (foo) (define else #f) (cond (else 1)))
(foo) => 1

(define (foo) (letrec ((else #f)) (cond (else 1))))
(foo) => ; #<unspecified>

According to R5RS, this should behave the same
http://www.schemers.org/Documents/Standards/R5RS/HTML/r5rs-Z-H-8.html#%_sec_5.2.2

comment:9 Changed 10 years ago by Moritz Heidkamp

Description: modified (diff)

Oops, the original problem reproduction code contained the (except chicken void) work-around which lead to it not reproducing the problem anymore. I fixed this now!

Changed 10 years ago by sjamaan

Attachment: ir-fix.patch added

Fix for IR renaming bug

comment:10 Changed 10 years ago by sjamaan

Here's a fix plus test. I'm still not 100% sure whether this qualifies as a bug, though.

I tried to add a second list "irenv" which was the inverse of "renv", but for some reason as soon as I assigned something to that list it caused a segfault I could not explain, so instead I just write a reverse-lookup procedure. May want to look at that.

comment:11 Changed 10 years ago by sjamaan

Owner: set to felix winkelmann
Status: newassigned

comment:12 Changed 10 years ago by sjamaan

Here's a testcase that also fails with the other macro system, plus a patch that fixes it. It causes strip-syntax to strip syntax off of primitives too. This is neccessary since macro syntax environments inside modules always contain all imported bindings, which includes {{(void . #%void)}}. This is always rewritten for syntax-rules and stripping syntax should restore this back to the original symbol "void".

Applying this patch also fixes the testcase in the other patch. I think both patches should be applied, probably. I can't currently think of a case which would break but I'm sure with deeply nested macros the earlier patch provides some added robustness.

Changed 10 years ago by sjamaan

Attachment: nested-syntax.patch added

Fix for nested ER macros/syntax-rules as well as IR macros

comment:13 Changed 10 years ago by felix winkelmann

Resolution: fixed
Status: assignedclosed

patch is applied, thanks.

comment:14 Changed 9 years ago by felix winkelmann

Milestone: 4.7.04.8.0

Milestone 4.7.0 deleted

comment:15 Changed 8 years ago by felix winkelmann

Milestone: 4.8.04.9.0

Milestone 4.8.0 deleted

Note: See TracTickets for help on using tickets.