Hi all, Here's a reasonably straightforward patch (I hope) for #1757. Commit should speak for itself, even if the stuff itself is a bit tricky.
I'm not 100% sure about the other places where "sexps" is used, if we should use the local macros only there, or not. All the tests seem to work if we keep it referring to only local macros, and I think we shouldn't mess too much with the environments where that's not needed, so I think it's fine. Cheers, Peter
From 96827b77554e8b56838b0e473633260118888b6e Mon Sep 17 00:00:00 2001
From: Peter Bex <pe...@more-magic.net>
Date: Wed, 14 Jul 2021 12:18:56 +0200
Subject: [PATCH] Don't merge syntactic environment for reexported macros
(#1757)
Separate the syntactic exports into local and reexported macros,
so we can merge the syntactic environment into the local macros.
The original code would merge the reexporting module's syntactic
environment into every macro it exports, but this is incorrect for
reexported macros - those should stick with their original syntactic
environment, as it was in the defining module.
---
NEWS | 4 ++++
distribution/manifest | 2 ++
modules.scm | 14 ++++++++------
tests/reexport-m7.scm | 17 +++++++++++++++++
tests/reexport-m8.scm | 8 ++++++++
tests/reexport-tests-2.scm | 6 ++++++
tests/runtests.bat | 4 ++++
tests/runtests.sh | 2 ++
8 files changed, 51 insertions(+), 6 deletions(-)
create mode 100644 tests/reexport-m7.scm
create mode 100644 tests/reexport-m8.scm
diff --git a/NEWS b/NEWS
index e65f885a..e0e2147d 100644
--- a/NEWS
+++ b/NEWS
@@ -33,6 +33,10 @@
- Made topological-sort behave better when dependency target is listed
multiple times by concatenating dependencies (fixes #1185).
+- Module system
+ - Reexported macros now work when the reexporting module redefines
+ identifiers from the original (fixes #1757, reported by Sandra Snan).
+
- Runtime system
- Sleeping primordial thread doesn't forget mutations made to
parameters in interrupt handlers anymore. (See #1638. Fix
diff --git a/distribution/manifest b/distribution/manifest
index 5416eff7..53aeaa4f 100644
--- a/distribution/manifest
+++ b/distribution/manifest
@@ -196,6 +196,8 @@ tests/reexport-m3.scm
tests/reexport-m4.scm
tests/reexport-m5.scm
tests/reexport-m6.scm
+tests/reexport-m7.scm
+tests/reexport-m8.scm
tests/reexport-tests.scm
tests/reexport-tests-2.scm
tests/ec.scm
diff --git a/modules.scm b/modules.scm
index 0ba1df49..77c9af52 100644
--- a/modules.scm
+++ b/modules.scm
@@ -385,16 +385,18 @@
'import "cannot find implementation of re-exported syntax"
name))))
(let* ((sexps
- (map (lambda (se)
- (if (symbol? se)
- (find-reexport se)
- (list (car se) #f (##sys#ensure-transformer (cdr se) (car se)))))
- sexports))
+ (filter-map (lambda (se)
+ (and (not (symbol? se))
+ (list (car se) #f (##sys#ensure-transformer (cdr se) (car se)))))
+ sexports))
+ (reexp-sexps
+ (filter-map (lambda (se) (and (symbol? se) (find-reexport se)))
+ sexports))
(nexps
(map (lambda (ne)
(list (car ne) #f (##sys#ensure-transformer (cdr ne) (car ne))))
sdefs))
- (mod (make-module name lib '() vexports sexps iexports))
+ (mod (make-module name lib '() vexports (append sexps reexp-sexps) iexports))
(senv (if (or (not (null? sexps)) ; Only macros have an senv
(not (null? nexps))) ; which must be patched up
(merge-se
diff --git a/tests/reexport-m7.scm b/tests/reexport-m7.scm
new file mode 100644
index 00000000..3c62fabf
--- /dev/null
+++ b/tests/reexport-m7.scm
@@ -0,0 +1,17 @@
+(module reexport-m7
+ (reexported-reverse (reexported-local-reverse my-reverse))
+
+ (import scheme (chicken syntax))
+
+ (define (my-reverse lis)
+ (reverse lis))
+
+ (define-syntax reexported-local-reverse
+ (syntax-rules ()
+ ((_ lis)
+ (my-reverse lis))))
+
+ (define-syntax reexported-reverse
+ (syntax-rules ()
+ ((_ lis)
+ (reverse lis)))))
diff --git a/tests/reexport-m8.scm b/tests/reexport-m8.scm
new file mode 100644
index 00000000..2fe2b9ac
--- /dev/null
+++ b/tests/reexport-m8.scm
@@ -0,0 +1,8 @@
+(module reexport-m8 ()
+ ;; NOTE: Reexport only works properly if m7 is imported here, when
+ ;; the implementing library isn't required by the user of m8..
+ (import reexport-m7
+ (rename scheme (reverse reverse-og))
+ (rename (chicken base) (identity reverse))
+ (chicken module))
+ (reexport reexport-m7))
diff --git a/tests/reexport-tests-2.scm b/tests/reexport-tests-2.scm
index 14b51c5a..61d81993 100644
--- a/tests/reexport-tests-2.scm
+++ b/tests/reexport-tests-2.scm
@@ -6,3 +6,9 @@
(import reexport-m6)
(f:s1) ; expands to s2, which is reexported and refers to "s2", which is also visible in this context as "f:s2"
(f:s2)
+
+;; reexport of syntax using shadowed identifiers in new module (#1757)
+(import reexport-m8)
+(assert (equal? '(d c b a) (reexported-reverse '(a b c d))))
+(assert (equal? '(d c b a) (reexported-local-reverse '(a b c d))))
+
diff --git a/tests/runtests.bat b/tests/runtests.bat
index 4db57040..b696899c 100644
--- a/tests/runtests.bat
+++ b/tests/runtests.bat
@@ -330,6 +330,10 @@ if errorlevel 1 exit /b 1
if errorlevel 1 exit /b 1
%compile_s% reexport-m6.scm -J
if errorlevel 1 exit /b 1
+%compile_s% reexport-m7.scm -J
+if errorlevel 1 exit /b 1
+%compile_s% reexport-m8.scm -J
+if errorlevel 1 exit /b 1
%compile% reexport-tests-2.scm
if errorlevel 1 exit /b 1
a.out
diff --git a/tests/runtests.sh b/tests/runtests.sh
index c99cbccc..11503b3c 100755
--- a/tests/runtests.sh
+++ b/tests/runtests.sh
@@ -262,6 +262,8 @@ $compile_s reexport-m3.scm -J
$compile_s reexport-m4.scm -J
$compile_s reexport-m5.scm -J
$compile_s reexport-m6.scm -J
+$compile_s reexport-m7.scm -J
+$compile_s reexport-m8.scm -J
$compile reexport-tests-2.scm
./a.out
--
2.20.1
signature.asc
Description: PGP signature
