Hi all, This ticket was reported a few days ago, and it's a reasonably easy fix. When looking up an identifier, we look it up in the current syntactic environment (SE) and if that fails, we check its '##core#macro-alias property, if any, and return it.
This behaviour is fine when looking up identifiers: it may have been renamed, so we need to look up its actual value. But when renaming, this is not fine, because that effectively _undoes_ the rename (if it was renamed before). So instead of using "lookup", rename now performs just the alist lookup itself. This bug affects master as well, so I think it's a good idea to fix it there too (see attached patches, there was a small conflict so I added patches for both versions). Cheers, Peter
From 05df2f8fdb852ee3ce6f20857ee41a4f57ceef9d Mon Sep 17 00:00:00 2001
From: Peter Bex <pe...@more-magic.net>
Date: Fri, 14 Apr 2017 16:49:54 +0200
Subject: [PATCH 1/2] Do not undo macro renaming when renaming twice.
The "lookup" procedure will try to look up the renamed identifier in
the syntax env and if that fails, it checks whether the looked-up
identifier is an alias of another identifier, in which case it
returns the original identifier.
This fallback behaviour causes problems when we rename an identifier
twice. In normal use, that doesn't tend to happen (but it could).
When using IR macros, this happens all the time, because the input
form is completely renamed first, and then the output form is
reverse-renamed (renaming any "plain" identifiers in the process).
This behaviour will "collapse" any plain identifiers, so that, on the
reverse rename, they are mapped to the same identifier as those from
the input form.
This fixes ticket #1362, as found by Megane.
---
NEWS | 4 ++++
expand.scm | 10 +++++-----
tests/syntax-tests.scm | 30 ++++++++++++++++++++++++++++++
3 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/NEWS b/NEWS
index df1af55..e06bd79 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,10 @@
- Unit "posix": If file-lock, file-lock/blocking or file-unlock are
interrupted by a signal, we now retry (thanks to Joerg Wittenberger).
+- Syntax expander
+ - Renaming an identifier twice no longer results in an undo of the
+ rename (fixes #1362, thanks to "megane").
+
- Build system
- Fixed broken compilation on NetBSD, due to missing _NETBSD_SOURCE.
diff --git a/expand.scm b/expand.scm
index 8020be3..55e3107 100644
--- a/expand.scm
+++ b/expand.scm
@@ -243,7 +243,7 @@
"syntax transformer for `" (symbol->string name)
"' returns original form, which would result in endless expansion")
exp))
- (dx `(,name --> ,exp2))
+ (dx `(,name ~~> ,exp2))
exp2)))
(define (expand head exp mdef)
(dd `(EXPAND:
@@ -813,7 +813,7 @@
(lambda (a)
(dd `(RENAME/RENV: ,sym --> ,(cdr a)))
(cdr a)))
- ((lookup sym se) =>
+ ((assq sym se) =>
(lambda (a)
(cond ((symbol? a)
;; Add an extra level of indirection for already aliased
@@ -822,15 +822,15 @@
(cond ((or (getp a '##core#aliased)
(getp a '##core#primitive))
(let ((a2 (macro-alias sym se)))
- (dd `(RENAME/LOOKUP/ALIASED: ,sym --> ,a ==> ,a2))
+ (dd `(RENAME/SE/ALIASED: ,sym --> ,a ==> ,a2))
(set! renv (cons (cons sym a2) renv))
a2))
- (else (dd `(RENAME/LOOKUP: ,sym --> ,a))
+ (else (dd `(RENAME/SE: ,sym --> ,a))
(set! renv (cons (cons sym a) renv))
a)))
(else
(let ((a2 (macro-alias sym se)))
- (dd `(RENAME/LOOKUP/MACRO: ,sym --> ,a2))
+ (dd `(RENAME/SE/MACRO: ,sym --> ,a2))
(set! renv (cons (cons sym a2) renv))
a2)))))
(else
diff --git a/tests/syntax-tests.scm b/tests/syntax-tests.scm
index 1004f71..e23f8af 100644
--- a/tests/syntax-tests.scm
+++ b/tests/syntax-tests.scm
@@ -1115,6 +1115,36 @@ take
(import rename-builtins)
(assert (eq? '* (strip-syntax-on-*)))
+;; #1362: Double rename would cause "renamed" var to be restored to
+;; the original macro aliased name (resulting in a plain symbol)
+(let-syntax ((wrapper/should-do-nothing
+ (er-macro-transformer
+ (lambda (e r c)
+ (let* ((%x (r 'x))
+ (%%x (r %x)))
+ `(let ((,%x 1)
+ (,%%x 2))
+ ,(cadr e)))))))
+ (print (let ((x 1)) (wrapper/should-do-nothing x))))
+
+;; Same net effect as above, but more complex by the use of IR macros.
+(letrec-syntax ((bind-pair
+ (ir-macro-transformer
+ (lambda (e i c)
+ (let* ((b (cadr e))
+ (exp (caddr e))
+ (body (cdddr e)))
+ `(let* ((x ,exp)
+ (,(car b) (car x))
+ (,(cadr b) (cdr x)))
+ ,@body)))))
+ (foo
+ (ir-macro-transformer
+ (lambda (e i c)
+ `(bind-pair (x y) (cons 'foo-car 'foo-cdr) y)))))
+ (assert (eq? 'second (bind-pair (x y) (cons 'first 'second) y)))
+ (assert (eq? 'foo-cdr (foo))))
+
;; #944: macro-renamed defines mismatch with the names recorded in module
;; definitions, causing the module to be unresolvable.
--
2.1.4
From bc58c0b635e71ec649255c0ad6fbfd0f266c838d Mon Sep 17 00:00:00 2001 From: Peter Bex <pe...@more-magic.net> Date: Fri, 14 Apr 2017 17:17:45 +0200 Subject: [PATCH 2/2] Remove unnecessary double alias for already aliased renamed identifiers. This was an overly complicated solution for #518, which was only necessary because of the bogus "lookup" call when renaming. Now that we don't undo a macro-alias on renaming anymore, but only look up already-renamed identifiers in the syntax env, this case is no longer triggered, so we fall back to the "else" clause, which will cause the symbol to be aliased normally. All the sample code snippets from that ticket behave identically with and without this patch. --- expand.scm | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/expand.scm b/expand.scm index 55e3107..9e73e09 100644 --- a/expand.scm +++ b/expand.scm @@ -816,18 +816,9 @@ ((assq sym se) => (lambda (a) (cond ((symbol? a) - ;; Add an extra level of indirection for already aliased - ;; symbols. This prevents aliased symbols from popping up - ;; in syntax-stripped output. - (cond ((or (getp a '##core#aliased) - (getp a '##core#primitive)) - (let ((a2 (macro-alias sym se))) - (dd `(RENAME/SE/ALIASED: ,sym --> ,a ==> ,a2)) - (set! renv (cons (cons sym a2) renv)) - a2)) - (else (dd `(RENAME/SE: ,sym --> ,a)) - (set! renv (cons (cons sym a) renv)) - a))) + (dd `(RENAME/SE: ,sym --> ,a)) + (set! renv (cons (cons sym a) renv)) + a) (else (let ((a2 (macro-alias sym se))) (dd `(RENAME/SE/MACRO: ,sym --> ,a2)) -- 2.1.4
From 363d5ec6d9c49fe25907cbf8496bb786fe402324 Mon Sep 17 00:00:00 2001
From: Peter Bex <pe...@more-magic.net>
Date: Fri, 14 Apr 2017 16:49:54 +0200
Subject: [PATCH 1/2] Do not undo macro renaming when renaming twice.
The "lookup" procedure will try to look up the renamed identifier in
the syntax env and if that fails, it checks whether the looked-up
identifier is an alias of another identifier, in which case it
returns the original identifier.
This fallback behaviour causes problems when we rename an identifier
twice. In normal use, that doesn't tend to happen (but it could).
When using IR macros, this happens all the time, because the input
form is completely renamed first, and then the output form is
reverse-renamed (renaming any "plain" identifiers in the process).
This behaviour will "collapse" any plain identifiers, so that, on the
reverse rename, they are mapped to the same identifier as those from
the input form.
This fixes ticket #1362, as found by Megane.
Conflicts:
expand.scm
---
NEWS | 4 ++++
expand.scm | 10 +++++-----
tests/syntax-tests.scm | 30 ++++++++++++++++++++++++++++++
3 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/NEWS b/NEWS
index 1d59f4d..e0668c7 100644
--- a/NEWS
+++ b/NEWS
@@ -72,6 +72,10 @@
- Unit "posix": If file-lock, file-lock/blocking or file-unlock are
interrupted by a signal, we now retry (thanks to Joerg Wittenberger).
+- Syntax expander
+ - Renaming an identifier twice no longer results in an undo of the
+ rename (fixes #1362, thanks to "megane").
+
- Build system
- Fixed broken compilation on NetBSD, due to missing _NETBSD_SOURCE.
diff --git a/expand.scm b/expand.scm
index 02e69de..b71a4de 100644
--- a/expand.scm
+++ b/expand.scm
@@ -259,7 +259,7 @@
"syntax transformer for `" (symbol->string name)
"' returns original form, which would result in endless expansion")
exp))
- (dx `(,name --> ,exp2))
+ (dx `(,name ~~> ,exp2))
(expansion-result-hook exp exp2) ) ) )
(define (expand head exp mdef)
(dd `(EXPAND:
@@ -842,7 +842,7 @@
(lambda (a)
(dd `(RENAME/RENV: ,sym --> ,(cdr a)))
(cdr a)))
- ((lookup sym se) =>
+ ((assq sym se) =>
(lambda (a)
(cond ((symbol? a)
;; Add an extra level of indirection for already aliased
@@ -851,15 +851,15 @@
(cond ((or (getp a '##core#aliased)
(getp a '##core#primitive))
(let ((a2 (macro-alias sym se)))
- (dd `(RENAME/LOOKUP/ALIASED: ,sym --> ,a ==> ,a2))
+ (dd `(RENAME/SE/ALIASED: ,sym --> ,a ==> ,a2))
(set! renv (cons (cons sym a2) renv))
a2))
- (else (dd `(RENAME/LOOKUP: ,sym --> ,a))
+ (else (dd `(RENAME/SE: ,sym --> ,a))
(set! renv (cons (cons sym a) renv))
a)))
(else
(let ((a2 (macro-alias sym se)))
- (dd `(RENAME/LOOKUP/MACRO: ,sym --> ,a2))
+ (dd `(RENAME/SE/MACRO: ,sym --> ,a2))
(set! renv (cons (cons sym a2) renv))
a2)))))
(else
diff --git a/tests/syntax-tests.scm b/tests/syntax-tests.scm
index 49f9d64..a803220 100644
--- a/tests/syntax-tests.scm
+++ b/tests/syntax-tests.scm
@@ -1135,6 +1135,36 @@ other-eval
(import rename-builtins)
(assert (eq? '* (strip-syntax-on-*)))
+;; #1362: Double rename would cause "renamed" var to be restored to
+;; the original macro aliased name (resulting in a plain symbol)
+(let-syntax ((wrapper/should-do-nothing
+ (er-macro-transformer
+ (lambda (e r c)
+ (let* ((%x (r 'x))
+ (%%x (r %x)))
+ `(let ((,%x 1)
+ (,%%x 2))
+ ,(cadr e)))))))
+ (print (let ((x 1)) (wrapper/should-do-nothing x))))
+
+;; Same net effect as above, but more complex by the use of IR macros.
+(letrec-syntax ((bind-pair
+ (ir-macro-transformer
+ (lambda (e i c)
+ (let* ((b (cadr e))
+ (exp (caddr e))
+ (body (cdddr e)))
+ `(let* ((x ,exp)
+ (,(car b) (car x))
+ (,(cadr b) (cdr x)))
+ ,@body)))))
+ (foo
+ (ir-macro-transformer
+ (lambda (e i c)
+ `(bind-pair (x y) (cons 'foo-car 'foo-cdr) y)))))
+ (assert (eq? 'second (bind-pair (x y) (cons 'first 'second) y)))
+ (assert (eq? 'foo-cdr (foo))))
+
;; #944: macro-renamed defines mismatch with the names recorded in module
;; definitions, causing the module to be unresolvable.
--
2.1.4
From 7bfdddc49b7d58963863679af766430b1692bfe7 Mon Sep 17 00:00:00 2001 From: Peter Bex <pe...@more-magic.net> Date: Fri, 14 Apr 2017 17:17:45 +0200 Subject: [PATCH 2/2] Remove unnecessary double alias for already aliased renamed identifiers. This was an overly complicated solution for #518, which was only necessary because of the bogus "lookup" call when renaming. Now that we don't undo a macro-alias on renaming anymore, but only look up already-renamed identifiers in the syntax env, this case is no longer triggered, so we fall back to the "else" clause, which will cause the symbol to be aliased normally. All the sample code snippets from that ticket behave identically with and without this patch. --- expand.scm | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/expand.scm b/expand.scm index b71a4de..3a6b436 100644 --- a/expand.scm +++ b/expand.scm @@ -845,18 +845,9 @@ ((assq sym se) => (lambda (a) (cond ((symbol? a) - ;; Add an extra level of indirection for already aliased - ;; symbols. This prevents aliased symbols from popping up - ;; in syntax-stripped output. - (cond ((or (getp a '##core#aliased) - (getp a '##core#primitive)) - (let ((a2 (macro-alias sym se))) - (dd `(RENAME/SE/ALIASED: ,sym --> ,a ==> ,a2)) - (set! renv (cons (cons sym a2) renv)) - a2)) - (else (dd `(RENAME/SE: ,sym --> ,a)) - (set! renv (cons (cons sym a) renv)) - a))) + (dd `(RENAME/SE: ,sym --> ,a)) + (set! renv (cons (cons sym a) renv)) + a) (else (let ((a2 (macro-alias sym se))) (dd `(RENAME/SE/MACRO: ,sym --> ,a2)) -- 2.1.4
signature.asc
Description: Digital signature
_______________________________________________ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers
