Hi all, While talking to Demosthenex in #chicken, I decided it would be cool to compare the performance of the "json" egg in CHICKEN 5 with that in CHICKEN 4. However, I got some errors when building the egg, which have to do with the way the "packrat" egg expands its bodies.
It turns out that packrat-parser expands into this sort of pattern:
(let ()
(define nonterminal1 (lambda (results) ...))
(define nonterminal2 (lambda (results) ...))
body)
And in json.scm, the json-read parser uses something like
(packrat-parser (begin (define (helper1 results) ...)
(define (helper2 results) ...)
the-parser)
(the-parser ((helper1 ...))))
So the final output is like
(let ()
(define parse (lambda (results) (helper1 ..)))
(begin (define (helper1 results) ...)
(define (helper2 results) ...)
parse))
And if the begin starts a new letrec, this causes the helpers
to be unavailable from "parse".
Now, the spec disallows this (I think..), but we typically treat
begin as a sort of no-op, so in the above example, all 3 defines
should be in one letrec sequence. I also noticed that if we
create a macro that's basically an alias for define, it starts
a new letrec.
This is all due a simplification I added in c9220247d that I thought
shouldn't cause problems: I removed the macro-expansion in the main
loop of canonicalize-body because "there already was one in the
secondary loop" (and the check for the macro being a member of
"vars" seemed a bit iffy to me as well). Technically that's true,
but I hadn't thought through the implications of this resulting in
a new letrec.
So attached is a relatively simple patch which restores the old macro
expansion behaviour in canonicalize-body, plus two regression tests
for these situations.
PS: It turns out the json egg was slightly slower to parse the benchmark
file with CHICKEN 5 than with CHICKEN 4 but it used less memory.
Cheers,
Peter
From 144b11ecc6b1d5984be7a99e3141b3af9bce9869 Mon Sep 17 00:00:00 2001
From: Peter Bex <pe...@more-magic.net>
Date: Thu, 27 Apr 2017 17:43:13 +0200
Subject: [PATCH] Restore macro-expansion in canonicalize-body's "main" loop
In commit c9220247dbcdf6fd39697b428cfd40068244219a, we removed a
little bit too much: the original code's expansion in the main loop
would expand begins and defines at the root level, which means they'd
be "flattened" into the same letrec.
This broke a few situations that are technically somewhat iffy, like
(let () (define (foo) bar) (begin (define (bar) 1) (foo)))
which would error out instead of returning 1, because the begin would
stop the main loop and inject a new letrec, acting as a barrier, which
means later definitions are in a new scope that the earlier
definitions would not be able to see.
Also, if there's an macro that expands to "define", that would also be
put in its own letrec because macro-expansion would stop the current
collection of vars and mvars, much like the aforementioned begin.
---
expand.scm | 11 ++++++++++-
tests/syntax-tests.scm | 6 ++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/expand.scm b/expand.scm
index 3a6b436..472d3ca 100644
--- a/expand.scm
+++ b/expand.scm
@@ -630,7 +630,16 @@
(loop rest (cons (cadr x) vars) (cons (caddr x) vals) (cons #t mvars)))
((comp '##core#begin head)
(loop (##sys#append (cdr x) rest) vars vals mvars))
- (else (fini vars vals mvars body))))))))
+ (else
+ ;; Do not macro-expand local definitions we are
+ ;; in the process of introducing.
+ (if (member (list head) vars)
+ (fini vars vals mvars body)
+ (let ((x2 (##sys#expand-0 x se cs?)))
+ (if (eq? x x2)
+ (fini vars vals mvars body)
+ (loop (cons x2 rest)
+ vars vals mvars)))))))))))
(expand body) ) )
diff --git a/tests/syntax-tests.scm b/tests/syntax-tests.scm
index a803220..1c4941a 100644
--- a/tests/syntax-tests.scm
+++ b/tests/syntax-tests.scm
@@ -797,6 +797,12 @@
;; Nested begins inside definitions were not treated correctly
(t 3 (eval '(let () (begin 1 (begin 2 (define internal-def 3) internal-def)))))
+;; Macros that expand to "define" should not cause a letrec barrier
+(t 1 (eval '(let-syntax ((my-define (syntax-rules ()
+ ((_ var val) (define var val)))))
+ (let () (define (run-it) foo) (my-define foo 1) (run-it)))))
+;; Begin should not cause a letrec barrier
+(t 1 (eval '(let () (define (run-it) foo) (begin (define foo 1) (run-it)))))
(f (eval '(let () internal-def)))
;;; renaming of keyword argument (#277)
--
2.1.4
signature.asc
Description: Digital signature
_______________________________________________ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers
