reexport with renaming fails when import libraries are used

[felix winkelmann]

In the following code s2 in the expansion of s1 will not be found when foo1 is used via an import library:

(module foo *
(import scheme)
(define-syntax s1 
  (syntax-rules () ((_) (s2))))
(define-syntax s2 
  (syntax-rules () ((_) (display 1)))))

(module foo1 ()
(reexport (prefix foo f:)))

(import foo1)
(f:s1)

[sjamaan]

After some investigation, it became clear that the set-car! calls in the foreaches of {{##sys#register-compiled-module}} are clobbering the carefully saved syntax environments of each macro.

In the example, when {{foo1}} is imported and the {{find-reexport}} helper procedure in {{##sys#register-compiled-module}} finds the matching original binding ({{s1}} for {{f1:s1}}) that's being re-exported, it has a syntax environment containing both {{s1}} and {{s2}}.
You can see this in action if you apply this debugging patch:

diff --git a/modules.scm b/modules.scm
index b1571c2..4c78320 100644
--- a/modules.scm
+++ b/modules.scm
@@ -33,7 +33,7 @@
 
 (include "common-declarations.scm")
 
-(define-syntax d (syntax-rules () ((_ . _) (void))))
+#;(define-syntax d (syntax-rules () ((_ . _) (void))))
 
 (define-alias dd d)
 (define-alias dm d)
@@ -347,7 +347,7 @@
   (define (find-reexport name)
     (let ((a (assq name (##sys#macro-environment))))
       (if (and a (pair? (cdr a)))
-	  a
+	  (begin (dm `(FOUND-REEXP: ,(car a) ---> ,(map-se (cadr a)))) a)
 	  (##sys#error
 	   'import "cannot find implementation of re-exported syntax"
 	   name))))

However, after the module is register, if you inspect {{##sys#module-environment}}, the syntax environment attached to {{f1:s1}} contains {{f1:s1}} and {{f1:s2}}. It's been replaced by the foreach loops.

I'm unsure how to fix this. What's the reason these environments are reset the way they are?

[sjamaan]

Damn you, Trac syntax

[felix winkelmann]

I think you are on the right track, sjamaan. I have modified ##sys#register-compiled-module to merge the new SE with the existing SE for each import, instead of blindly overwriting it. I'm not completely sure whether this is necessary for all imports, but so far it seems to work.

I have added a test-case and all other tests seem to run ok. I have not attempted a full bootstrap, but since chicken itself doesn't use such deep module magic, this shouldn't be a problem.

A patch is attached.

[sjamaan]

Fixed by 4777fb07523457614296e3a1d037f5d8266f464d