On Sat, Feb 25, 2017 at 06:13:48PM +0100, Peter Bex wrote: > However, I figured out that the real cause of #1309 is much, much deeper > and has to do with a nasty bug in ##sys#canonicalize-body: it invokes > "fini" as soon as it sees a non-pair. However, fini doesn't handle > expansions the same way as the body. For example: > > (let () 1 (begin (define blabla 3)) blabla) > > This will define blabla globally (or trigger an error in CHICKEN 5 with > the patch), because the "1" stops the begin from being expanded properly. > I fear that this requires a pretty invasive rewrite of > ##sys#canonicalize-body. I'll get back to y'all on that.
Attached is an updated and hopefully final patch set to tackle this bug. The fix for the nested "begin" mentioned above is pretty simple: The list of special cases in "fini" needs to be extended with ##core#begin; if that is encountered, we need to call "expand" again, which will flatten all the begins. But the trickier bit here is that the forms are all macro-expanded by ##sys#canonicalize-body. This results in three problems: 1) A form that expands to something that eventually expands to "define" was expanded just once and then processed by the compiler as a regular form, resulting in a toplevel define, which is incorrect. 2) After fixing the above by expanding macros completely in ##sys#canonicalize-body, line numbers are lost in the compiler because ##sys#canonicalize-body does not have access to the line number database maintained by the compiler. 3) "import" and "module" need special consideration with macro expansion. Here, number 1) was fixed by simplifying macro expansion a bit: the "expand" procedure no longer tries to do macro-expansion or detection of calls to any locally defined procedures or macros. Instead, "fini" now does this. It will expand until it finds a define, "begin", "import" or "module", and then either hand back control to the compiler or re-invoke "expand" on the remaining body to grab all the defines it can find. Number 2) was easily fixed by adding yet another hook. It's ugly, but I've discussed this on IRC with Felix and he sess no other way to do it either. The advantage is that this actually _improves_ precision of line numbers, as you can see in the final patch's hunk that changes the scrutiny-2.expected; it now correctly knows that the (pair?) call is on line 20 instead of on line 14 (which is the start of the LET). Number 3) deserves some more attention. It turns out that a (module) form which instantiates a functor will expand to two (module ...) forms, one for an "internal" module and one for another module that imports this internal one. However, the macro will look up the other module at expansion time, in a table that's only populated once the ##core#module form is processed by the compiler or interpreter core. Thus, if you expand both module forms one by one, this will fail. The other issue is with "import". In functor-tests, there is a test that creates three different functors, which are then used in an inner define: (define output (with-output-to-string (lambda () (import (2x print)) (print-twice #\a) (import (2x noop)) (print-twice #\a) (import (2x write)) (print-twice #\a)))) This is supposed to import (2x print) and use print-twice from that module. Then it will import module (2x noop) and use print-twice from there, then import module (2x write) and use its print-twice. This _requires_ that expansion of import is delayed. Normally, one would see an import as a "global action", affecting all subsequent uses of the identifiers from the imported module. This is exactly what happens if you expand the import, then process the rest of the body. So what needs to happen is to process all the forms, except for any imports. Then, the compiler can process the import, process the following form, process the import, process the form, etc to get the interleaving that was intended. This works in the above code in CHICKEN 4, but only "by accident". A simple change can already break this behaviour: (define output (with-output-to-string (lambda () (import (2x print)) (print-twice #\a) (import (2x noop)) (print-twice #\a) (define whatever 1) ;; This interferes with the final print (import (2x write)) (print-twice #\a)))) This causes the imports to be expanded incorrectly. All this is now handled correctly with the attached patches, including the r7rs test case from the original bugreport; the r7rs version of define-record-type is now correctly seen as an internal define. And truly incorrectly placed defines are now rejected with a syntax error, of course :) Cheers, Peter
From 7e51fc89adc74a775cc6b42892f84b51c70a247b Mon Sep 17 00:00:00 2001 From: Peter Bex <pe...@more-magic.net> Date: Sat, 11 Feb 2017 15:30:13 +0100 Subject: [PATCH 1/5] Reject toplevel definitions in non-toplevel contexts. This introduces a distinction between define and set!, which allows the compiler (and the closure-compiler in the interpreter) to error out when a definition somehow ends up out of place. Fixes #1309 --- NEWS | 2 + core.scm | 166 +++++++++++++++++++++++++++--------------------- eval.scm | 155 ++++++++++++++++++++++---------------------- expand.scm | 2 +- tests/functor-tests.scm | 2 + tests/syntax-tests.scm | 7 ++ 6 files changed, 187 insertions(+), 147 deletions(-) diff --git a/NEWS b/NEWS index 4c97bcf..aee94c5 100644 --- a/NEWS +++ b/NEWS @@ -57,6 +57,8 @@ - Syntax expander - Removed support for (define-syntax (foo e r c) ...), which was undocumented and not officially supported anyway. + - define and friends are now aggressively rejected in "expression + contexts" (i.e., anywhere but toplevel or as internal defines). 4.12.1 diff --git a/core.scm b/core.scm index 718e7e8..b24e5ca 100644 --- a/core.scm +++ b/core.scm @@ -110,6 +110,7 @@ ; (##core#lambda <variable> <body>) ; (##core#lambda ({<variable>}+ [. <variable>]) <body>) ; (##core#set! <variable> <exp>) +; (##core#define-toplevel <variable> <exp>) ; (##core#begin <exp> ...) ; (##core#include <string> <string> | #f) ; (##core#loop-lambda <llist> <body>) @@ -529,9 +530,9 @@ (d `(RESOLVE-VARIABLE: ,x0 ,x ,(map (lambda (x) (car x)) se))) (cond ((not (symbol? x)) x0) ; syntax? ((##sys#hash-table-ref constant-table x) - => (lambda (val) (walk val e se dest ldest h #f))) + => (lambda (val) (walk val e se dest ldest h #f #f))) ((##sys#hash-table-ref inline-table x) - => (lambda (val) (walk val e se dest ldest h #f))) + => (lambda (val) (walk val e se dest ldest h #f #f))) ((assq x foreign-variables) => (lambda (fv) (let* ((t (second fv)) @@ -541,7 +542,7 @@ (foreign-type-convert-result (finish-foreign-result ft body) t) - e se dest ldest h #f)))) + e se dest ldest h #f #f)))) ((assq x location-pointer-map) => (lambda (a) (let* ((t (third a)) @@ -551,7 +552,7 @@ (foreign-type-convert-result (finish-foreign-result ft body) t) - e se dest ldest h #f)))) + e se dest ldest h #f #f)))) ((##sys#get x '##core#primitive)) ((not (memq x e)) (##sys#alias-global-hook x #f h)) ; only if global (else x)))) @@ -579,7 +580,7 @@ (for-each pretty-print imps) (print "\n;; END OF FILE"))))) ) ) - (define (walk x e se dest ldest h outer-ln) + (define (walk x e se dest ldest h outer-ln tl?) (cond ((symbol? x) (cond ((keyword? x) `(quote ,x)) ((memq x unlikely-variables) @@ -603,22 +604,22 @@ (xexpanded (expand x se compiler-syntax-enabled))) (when ln (update-line-number-database! xexpanded ln)) (cond ((not (eq? x xexpanded)) - (walk xexpanded e se dest ldest h ln)) + (walk xexpanded e se dest ldest h ln tl?)) ((##sys#hash-table-ref inline-table name) => (lambda (val) - (walk (cons val (cdr x)) e se dest ldest h ln))) + (walk (cons val (cdr x)) e se dest ldest h ln #f))) (else (case name ((##core#if) `(if - ,(walk (cadr x) e se #f #f h ln) - ,(walk (caddr x) e se #f #f h ln) + ,(walk (cadr x) e se #f #f h ln #f) + ,(walk (caddr x) e se #f #f h ln #f) ,(if (null? (cdddr x)) '(##core#undefined) - (walk (cadddr x) e se #f #f h ln) ) ) ) + (walk (cadddr x) e se #f #f h ln #f) ) ) ) ((##core#syntax ##core#quote) `(quote ,(strip-syntax (cadr x)))) @@ -626,21 +627,21 @@ ((##core#check) (if unsafe ''#t - (walk (cadr x) e se dest ldest h ln) ) ) + (walk (cadr x) e se dest ldest h ln tl?) ) ) ((##core#the) `(##core#the ,(strip-syntax (cadr x)) ,(caddr x) - ,(walk (cadddr x) e se dest ldest h ln))) + ,(walk (cadddr x) e se dest ldest h ln tl?))) ((##core#typecase) `(##core#typecase ,(or ln (cadr x)) - ,(walk (caddr x) e se #f #f h ln) + ,(walk (caddr x) e se #f #f h ln tl?) ,@(map (lambda (cl) (list (strip-syntax (car cl)) - (walk (cadr cl) e se dest ldest h ln))) + (walk (cadr cl) e se dest ldest h ln tl?))) (cdddr x)))) ((##core#immutable) @@ -667,7 +668,7 @@ ((##core#inline_loc_ref) `(##core#inline_loc_ref ,(strip-syntax (cadr x)) - ,(walk (caddr x) e se dest ldest h ln))) + ,(walk (caddr x) e se dest ldest h ln #f))) ((##core#require-for-syntax) (load-extension (cadr x)) @@ -683,7 +684,7 @@ file-requirements type (cut lset-adjoin/eq? <> id) (cut list id))) - (walk exp e se dest ldest h ln)))) + (walk exp e se dest ldest h ln #f)))) ((##core#let) (let* ((bindings (cadr x)) @@ -693,12 +694,12 @@ (set-real-names! aliases vars) `(let ,(map (lambda (alias b) - (list alias (walk (cadr b) e se (car b) #t h ln)) ) + (list alias (walk (cadr b) e se (car b) #t h ln #f)) ) aliases bindings) ,(walk (##sys#canonicalize-body (cddr x) se2 compiler-syntax-enabled) (append aliases e) - se2 dest ldest h ln) ) ) ) + se2 dest ldest h ln #f) ) ) ) ((##core#letrec*) (let ((bindings (cadr x)) @@ -712,7 +713,7 @@ `(##core#set! ,(car b) ,(cadr b))) bindings) (##core#let () ,@body) ) - e se dest ldest h ln))) + e se dest ldest h ln #f))) ((##core#letrec) (let* ((bindings (cadr x)) @@ -730,7 +731,7 @@ `(##core#set! ,v ,t)) vars tmps) (##core#let () ,@body) ) ) - e se dest ldest h ln))) + e se dest ldest h ln #f))) ((##core#lambda) (let ((llist (cadr x)) @@ -753,7 +754,7 @@ (##core#debug-event "C_DEBUG_ENTRY" ',dest) ,body0) body0) - (append aliases e) se2 #f #f dest ln)) + (append aliases e) se2 #f #f dest ln #f)) (llist2 (build-lambda-list aliases argc @@ -790,7 +791,7 @@ (walk (##sys#canonicalize-body (cddr x) se2 compiler-syntax-enabled) e se2 - dest ldest h ln) ) ) + dest ldest h ln #f) ) ) ((##core#letrec-syntax) (let* ((ms (map (lambda (b) @@ -808,7 +809,7 @@ ms) (walk (##sys#canonicalize-body (cddr x) se2 compiler-syntax-enabled) - e se2 dest ldest h ln))) + e se2 dest ldest h ln #f))) ((##core#define-syntax) (##sys#check-syntax @@ -833,7 +834,7 @@ ',var (##sys#current-environment) ,body) ;XXX possibly wrong se? '(##core#undefined) ) - e se dest ldest h ln)) ) + e se dest ldest h ln #f)) ) ((##core#define-compiler-syntax) (let* ((var (cadr x)) @@ -865,7 +866,7 @@ ',var) (##sys#current-environment)))) '(##core#undefined) ) - e se dest ldest h ln))) + e se dest ldest h ln #f))) ((##core#let-compiler-syntax) (let ((bs (map @@ -892,7 +893,7 @@ (walk (##sys#canonicalize-body (cddr x) se compiler-syntax-enabled) - e se dest ldest h ln) ) + e se dest ldest h ln tl?) ) (lambda () (for-each (lambda (b) @@ -907,7 +908,7 @@ (cadr x) (caddr x) (lambda (forms) - (walk `(##core#begin ,@forms) e se dest ldest h ln))))) + (walk `(##core#begin ,@forms) e se dest ldest h ln tl?))))) ((##core#let-module-alias) (##sys#with-module-aliases @@ -916,7 +917,7 @@ (strip-syntax b)) (cadr x)) (lambda () - (walk `(##core#begin ,@(cddr x)) e se dest ldest h ln)))) + (walk `(##core#begin ,@(cddr x)) e se dest ldest h ln #t)))) ((##core#module) (let* ((name (strip-syntax (cadr x))) @@ -986,7 +987,7 @@ (car body) e ;? (##sys#current-environment) - #f #f h ln) + #f #f h ln #t) ; reset to toplevel! xs)))))))))) (let ((body (canonicalize-begin-body @@ -999,7 +1000,7 @@ (walk x e ;? - (##sys#current-meta-environment) #f #f h ln) ) + (##sys#current-meta-environment) #f #f h ln tl?) ) (cons `(##core#provide ,req) module-registration))) body)))) (do ((cs compiler-syntax (cdr cs))) @@ -1017,15 +1018,20 @@ (walk (##sys#canonicalize-body obody se2 compiler-syntax-enabled) (append aliases e) - se2 #f #f dest ln) ] ) + se2 #f #f dest ln #f) ] ) (set-real-names! aliases vars) `(##core#lambda ,aliases ,body) ) ) - ((##core#set!) + ((##core#set! ##core#define-toplevel) (let* ([var0 (cadr x)] [var (lookup var0 se)] [ln (get-line x)] [val (caddr x)] ) + (when (and (eq? name '##core#define-toplevel) (not tl?)) + (quit-compiling + "~atoplevel definition of `~s' in non-toplevel context" + (if ln (sprintf "(~a) - " ln) "") + var)) (when (memq var unlikely-variables) (warning (sprintf "assignment to variable `~s' possibly unintended" @@ -1039,7 +1045,7 @@ (##core#inline_update (,(third fv) ,type) ,(foreign-type-check tmp type) ) ) - e se #f #f h ln)))) + e se #f #f h ln #f)))) ((assq var location-pointer-map) => (lambda (a) (let* ([type (third a)] @@ -1050,7 +1056,7 @@ (,type) ,(second a) ,(foreign-type-check tmp type) ) ) - e se #f #f h ln)))) + e se #f #f h ln #f)))) (else (unless (memq var e) ; global? (set! var (or (##sys#get var '##core#primitive) @@ -1074,38 +1080,38 @@ (##sys#notice "assignment to imported value binding" var))) (when (keyword? var) (warning (sprintf "assignment to keyword `~S'" var) )) - `(set! ,var ,(walk val e se var0 (memq var e) h ln)))))) + `(set! ,var ,(walk val e se var0 (memq var e) h ln #f)))))) ((##core#debug-event) `(##core#debug-event ,(unquotify (cadr x) se) ,ln ; this arg is added - from this phase on ##core#debug-event has an additional argument! ,@(map (lambda (arg) - (unquotify (walk arg e se #f #f h ln) se)) + (unquotify (walk arg e se #f #f h ln tl?) se)) (cddr x)))) ((##core#inline) `(##core#inline - ,(unquotify (cadr x) se) ,@(mapwalk (cddr x) e se h ln))) + ,(unquotify (cadr x) se) ,@(mapwalk (cddr x) e se h ln #f))) ((##core#inline_allocate) `(##core#inline_allocate ,(map (cut unquotify <> se) (second x)) - ,@(mapwalk (cddr x) e se h ln))) + ,@(mapwalk (cddr x) e se h ln #f))) ((##core#inline_update) - `(##core#inline_update ,(cadr x) ,(walk (caddr x) e se #f #f h ln)) ) + `(##core#inline_update ,(cadr x) ,(walk (caddr x) e se #f #f h ln #f)) ) ((##core#inline_loc_update) `(##core#inline_loc_update ,(cadr x) - ,(walk (caddr x) e se #f #f h ln) - ,(walk (cadddr x) e se #f #f h ln)) ) + ,(walk (caddr x) e se #f #f h ln #f) + ,(walk (cadddr x) e se #f #f h ln #f)) ) ((##core#compiletimetoo ##core#elaborationtimetoo) (let ((exp (cadr x))) (##sys#eval/meta exp) - (walk exp e se dest #f h ln) ) ) + (walk exp e se dest #f h ln tl?) ) ) ((##core#compiletimeonly ##core#elaborationtimeonly) (##sys#eval/meta (cadr x)) @@ -1118,24 +1124,24 @@ (let ([x (car xs)] [r (cdr xs)] ) (if (null? r) - (list (walk x e se dest ldest h ln)) - (cons (walk x e se #f #f h ln) (fold r)) ) ) ) ) + (list (walk x e se dest ldest h ln tl?)) + (cons (walk x e se #f #f h ln tl?) (fold r)) ) ) ) ) '(##core#undefined) ) ) ((##core#foreign-lambda) - (walk (expand-foreign-lambda x #f) e se dest ldest h ln) ) + (walk (expand-foreign-lambda x #f) e se dest ldest h ln #f) ) ((##core#foreign-safe-lambda) - (walk (expand-foreign-lambda x #t) e se dest ldest h ln) ) + (walk (expand-foreign-lambda x #t) e se dest ldest h ln #f) ) ((##core#foreign-lambda*) - (walk (expand-foreign-lambda* x #f) e se dest ldest h ln) ) + (walk (expand-foreign-lambda* x #f) e se dest ldest h ln #f) ) ((##core#foreign-safe-lambda*) - (walk (expand-foreign-lambda* x #t) e se dest ldest h ln) ) + (walk (expand-foreign-lambda* x #t) e se dest ldest h ln #f) ) ((##core#foreign-primitive) - (walk (expand-foreign-primitive x) e se dest ldest h ln) ) + (walk (expand-foreign-primitive x) e se dest ldest h ln #f) ) ((##core#define-foreign-variable) (let* ((var (strip-syntax (second x))) @@ -1169,17 +1175,23 @@ (define ,ret ,(if (pair? (cdr conv)) (second conv) '##sys#values)) ) - e se dest ldest h ln) ) ] + e se dest ldest h ln #f) ) ] [else (register-foreign-type! name type) '(##core#undefined) ] ) ) ) ((##core#define-external-variable) - (let* ([sym (second x)] - [name (symbol->string sym)] - [type (third x)] - [exported (fourth x)] - [rname (make-random-name)] ) + (let* ((sym (second x)) + (ln (ln (get-line x))) + (name (symbol->string sym)) + (type (third x)) + (exported (fourth x)) + (rname (make-random-name)) ) + (unless tl? + (quit-compiling + "~aexternal variable definition of `~s' in non-toplevel context" + (if ln (sprintf "(~a) - " ln) "") + sym)) (unless exported (set! name (symbol->string (fifth x)))) (set! external-variables (cons (vector name type exported) external-variables)) (set! foreign-variables @@ -1212,16 +1224,23 @@ '() ) ,(if init (fifth x) (fourth x)) ) ) e (alist-cons var alias se) - dest ldest h ln) ) ) + dest ldest h ln #f) ) ) ((##core#define-inline) (let* ((name (second x)) - (val `(##core#lambda ,@(cdaddr x)))) + (val `(##core#lambda ,@(cdaddr x))) + (ln (get-line x))) + (unless tl? + (quit-compiling + "~ainline definition of `~s' in non-toplevel context" + (if ln (sprintf "(~a) - " ln) "") + name)) (##sys#hash-table-set! inline-table name val) '(##core#undefined))) ((##core#define-constant) (let* ((name (second x)) + (ln (get-line x)) (valexp (third x)) (val (handle-exceptions ex ;; could show line number here @@ -1233,6 +1252,11 @@ (eval `(##core#let ,defconstant-bindings ,valexp)))))) + (unless tl? + (quit-compiling + "~aconstant definition of `~s' in non-toplevel context" + (if ln (sprintf "(~a) - " ln) "") + name)) (set! defconstant-bindings (cons (list name `(##core#quote ,val)) defconstant-bindings)) (cond ((collapsable-literal? val) @@ -1244,7 +1268,7 @@ (hide-variable var) (mark-variable var '##compiler#constant) (mark-variable var '##compiler#always-bound) - (walk `(define ,var (##core#quote ,val)) e se #f #f h ln))) + (walk `(define ,var (##core#quote ,val)) e se #f #f h ln tl?))) (else (quit-compiling "invalid compile-time value for named constant `~S'" name))))) @@ -1258,7 +1282,7 @@ (lambda (id) (memq (lookup id se) e)))) (cdr x) ) ) - e '() #f #f h ln) ) + e '() #f #f h ln #f) ) ((##core#foreign-callback-wrapper) (let-values ([(args lam) (split-at (cdr x) 4)]) @@ -1280,7 +1304,7 @@ "non-matching or invalid argument list to foreign callback-wrapper" vars atypes) ) `(##core#foreign-callback-wrapper - ,@(mapwalk args e se h ln) + ,@(mapwalk args e se h ln #f) ,(walk `(##core#lambda ,vars (##core#let @@ -1337,7 +1361,7 @@ (##sys#make-c-string r ',name)) ) ) ) (else (cddr lam)) ) ) rtype) ) ) - e se #f #f h ln) ) ) ) ) + e se #f #f h ln #f) ) ) ) ) ((##core#location) (let ([sym (cadr x)]) @@ -1346,23 +1370,23 @@ => (lambda (a) (walk `(##sys#make-locative ,(second a) 0 #f 'location) - e se #f #f h ln) ) ] + e se #f #f h ln #f) ) ] [(assq sym external-to-pointer) - => (lambda (a) (walk (cdr a) e se #f #f h ln)) ] + => (lambda (a) (walk (cdr a) e se #f #f h ln #f)) ] [(assq sym callback-names) `(##core#inline_ref (,(symbol->string sym) c-pointer)) ] [else (walk `(##sys#make-locative ,sym 0 #f 'location) - e se #f #f h ln) ] ) + e se #f #f h ln #f) ] ) (walk `(##sys#make-locative ,sym 0 #f 'location) - e se #f #f h ln) ) ) ) + e se #f #f h ln #f) ) ) ) (else (let* ((x2 (fluid-let ((##sys#syntax-context (cons name ##sys#syntax-context))) - (mapwalk x e se h ln))) + (mapwalk x e se h ln tl?))) (head2 (car x2)) (old (##sys#hash-table-ref line-number-database-2 head2)) ) (when ln @@ -1378,7 +1402,7 @@ ((constant? (car x)) (emit-syntax-trace-info x #f) (warning "literal in operator position" x) - (mapwalk x e se h outer-ln) ) + (mapwalk x e se h outer-ln tl?) ) (else (emit-syntax-trace-info x #f) @@ -1387,10 +1411,10 @@ `(##core#let ((,tmp ,(car x))) (,tmp ,@(cdr x))) - e se dest ldest h outer-ln))))) + e se dest ldest h outer-ln #f))))) - (define (mapwalk xs e se h ln) - (map (lambda (x) (walk x e se #f #f h ln)) xs) ) + (define (mapwalk xs e se h ln tl?) + (map (lambda (x) (walk x e se #f #f h ln tl?)) xs) ) (when (memq 'c debugging-chicken) (newline) (pretty-print exp)) (foreign-code "C_clear_trace_buffer();") @@ -1403,7 +1427,7 @@ ,(begin (set! extended-bindings (append internal-bindings extended-bindings)) exp) ) - '() (##sys#current-environment) #f #f #f #f) ) ) + '() (##sys#current-environment) #f #f #f #f #t) ) ) (define (process-declaration spec se local?) diff --git a/eval.scm b/eval.scm index f1f6471..72977a8 100644 --- a/eval.scm +++ b/eval.scm @@ -207,7 +207,7 @@ (define compile-to-closure (let ((reverse reverse)) - (lambda (exp env se #!optional cntr evalenv static) + (lambda (exp env se #!optional cntr evalenv static tl?) (define (find-id id se) ; ignores macro bindings (cond ((null? se) #f) @@ -252,7 +252,7 @@ (define (decorate p ll h cntr) (eval-decorator p ll h cntr)) - (define (compile x e h tf cntr se) + (define (compile x e h tf cntr se tl?) (cond ((keyword? x) (lambda v x)) ((symbol? x) (receive (i j) (lookup x e se) @@ -318,7 +318,7 @@ (let ((x2 (expand x se))) (d `(EVAL/EXPANDED: ,x2)) (if (not (eq? x2 x)) - (compile x2 e h tf cntr se) + (compile x2 e h tf cntr se tl?) (let ((head (rename (##sys#slot x 0) se))) ;; here we did't resolve ##core#primitive, but that is done in compile-call (via ;; a normal walking of the operator) @@ -341,40 +341,42 @@ (lambda v c))) [(##core#check) - (compile (cadr x) e h tf cntr se) ] + (compile (cadr x) e h tf cntr se #f) ] [(##core#immutable) - (compile (cadr x) e #f tf cntr se) ] + (compile (cadr x) e #f tf cntr se #f) ] [(##core#undefined) (lambda (v) (##core#undefined))] [(##core#if) - (let* ([test (compile (cadr x) e #f tf cntr se)] - [cns (compile (caddr x) e #f tf cntr se)] - [alt (if (pair? (cdddr x)) - (compile (cadddr x) e #f tf cntr se) - (compile '(##core#undefined) e #f tf cntr se) ) ] ) + (let* ((test (compile (cadr x) e #f tf cntr se #f)) + (cns (compile (caddr x) e #f tf cntr se #f)) + (alt (if (pair? (cdddr x)) + (compile (cadddr x) e #f tf cntr se #f) + (compile '(##core#undefined) e #f tf cntr se #f) ) ) ) (lambda (v) (if (##core#app test v) (##core#app cns v) (##core#app alt v))) ) ] [(##core#begin) (let* ((body (##sys#slot x 1)) (len (length body)) ) (case len - [(0) (compile '(##core#undefined) e #f tf cntr se)] - [(1) (compile (##sys#slot body 0) e #f tf cntr se)] - [(2) (let* ([x1 (compile (##sys#slot body 0) e #f tf cntr se)] - [x2 (compile (cadr body) e #f tf cntr se)] ) - (lambda (v) (##core#app x1 v) (##core#app x2 v)) ) ] - [else - (let* ([x1 (compile (##sys#slot body 0) e #f tf cntr se)] - [x2 (compile (cadr body) e #f tf cntr se)] - [x3 (compile `(##core#begin ,@(##sys#slot (##sys#slot body 1) 1)) e #f tf cntr se)] ) - (lambda (v) (##core#app x1 v) (##core#app x2 v) (##core#app x3 v)) ) ] ) ) ] - - [(##core#set!) + ((0) (compile '(##core#undefined) e #f tf cntr se tl?)) + ((1) (compile (##sys#slot body 0) e #f tf cntr se tl?)) + ((2) (let* ([x1 (compile (##sys#slot body 0) e #f tf cntr se tl?)] + [x2 (compile (cadr body) e #f tf cntr se tl?)] ) + (lambda (v) (##core#app x1 v) (##core#app x2 v)) ) ) + (else + (let* ([x1 (compile (##sys#slot body 0) e #f tf cntr se tl?)] + [x2 (compile (cadr body) e #f tf cntr se tl?)] + [x3 (compile `(##core#begin ,@(##sys#slot (##sys#slot body 1) 1)) e #f tf cntr se tl?)] ) + (lambda (v) (##core#app x1 v) (##core#app x2 v) (##core#app x3 v)) ) ) ) ) ] + + [(##core#set! ##core#define-toplevel) (let ((var (cadr x))) + (when (and (eq? head '##core#define-toplevel) (not tl?)) + (##sys#error "toplevel definition in non-toplevel context for variable" var)) (receive (i j) (lookup var e se) - (let ((val (compile (caddr x) e var tf cntr se))) + (let ((val (compile (caddr x) e var tf cntr se #f))) (cond [(not i) (when ##sys#notices-enabled (and-let* ((a (assq var (##sys#current-environment))) @@ -406,28 +408,28 @@ (se2 (##sys#extend-se se vars aliases)) [body (compile-to-closure (##sys#canonicalize-body (cddr x) se2 #f) - e2 se2 cntr evalenv static) ] ) + e2 se2 cntr evalenv static #f) ] ) (case n - [(1) (let ([val (compile (cadar bindings) e (car vars) tf cntr se)]) + [(1) (let ([val (compile (cadar bindings) e (car vars) tf cntr se #f)]) (lambda (v) (##core#app body (cons (vector (##core#app val v)) v)) ) ) ] - [(2) (let ([val1 (compile (cadar bindings) e (car vars) tf cntr se)] - [val2 (compile (cadadr bindings) e (cadr vars) tf cntr se)] ) + [(2) (let ([val1 (compile (cadar bindings) e (car vars) tf cntr se #f)] + [val2 (compile (cadadr bindings) e (cadr vars) tf cntr se #f)] ) (lambda (v) (##core#app body (cons (vector (##core#app val1 v) (##core#app val2 v)) v)) ) ) ] - [(3) (let* ([val1 (compile (cadar bindings) e (car vars) tf cntr se)] - [val2 (compile (cadadr bindings) e (cadr vars) tf cntr se)] + [(3) (let* ([val1 (compile (cadar bindings) e (car vars) tf cntr se #f)] + [val2 (compile (cadadr bindings) e (cadr vars) tf cntr se #f)] [t (cddr bindings)] - [val3 (compile (cadar t) e (caddr vars) tf cntr se)] ) + [val3 (compile (cadar t) e (caddr vars) tf cntr se #f)] ) (lambda (v) (##core#app body (cons (vector (##core#app val1 v) (##core#app val2 v) (##core#app val3 v)) v)) ) ) ] - [(4) (let* ([val1 (compile (cadar bindings) e (car vars) tf cntr se)] - [val2 (compile (cadadr bindings) e (cadr vars) tf cntr se)] + [(4) (let* ([val1 (compile (cadar bindings) e (car vars) tf cntr se #f)] + [val2 (compile (cadadr bindings) e (cadr vars) tf cntr se #f)] [t (cddr bindings)] - [val3 (compile (cadar t) e (caddr vars) tf cntr se)] - [val4 (compile (cadadr t) e (cadddr vars) tf cntr se)] ) + [val3 (compile (cadar t) e (caddr vars) tf cntr se #f)] + [val4 (compile (cadadr t) e (cadddr vars) tf cntr se #f)] ) (lambda (v) (##core#app body @@ -437,7 +439,7 @@ (##core#app val4 v)) v)) ) ) ] [else - (let ([vals (map (lambda (x) (compile (cadr x) e (car x) tf cntr se)) bindings)]) + (let ((vals (map (lambda (x) (compile (cadr x) e (car x) tf cntr se #f)) bindings))) (lambda (v) (let ([v2 (##sys#make-vector n)]) (do ([i 0 (fx+ i 1)] @@ -458,7 +460,7 @@ `(##core#set! ,(car b) ,(cadr b))) bindings) (##core#let () ,@body) ) - e h tf cntr se))) + e h tf cntr se #f))) ((##core#letrec) (let* ((bindings (cadr x)) @@ -475,7 +477,7 @@ `(##core#set! ,v ,t)) vars tmps) (##core#let () ,@body) ) ) - e h tf cntr se))) + e h tf cntr se #f))) [(##core#lambda) (##sys#check-syntax 'lambda x '(_ lambda-list . #(_ 1)) #f se) @@ -496,7 +498,7 @@ (body (compile-to-closure (##sys#canonicalize-body body se2 #f) - e2 se2 (or h cntr) evalenv static) ) ) + e2 se2 (or h cntr) evalenv static #f) ) ) (case argc [(0) (if rest (lambda (v) @@ -583,7 +585,7 @@ se) ) ) (compile (##sys#canonicalize-body (cddr x) se2 #f) - e #f tf cntr se2))) + e #f tf cntr se2 #f))) ((##core#letrec-syntax) (let* ((ms (map (lambda (b) @@ -601,7 +603,7 @@ ms) (compile (##sys#canonicalize-body (cddr x) se2 #f) - e #f tf cntr se2))) + e #f tf cntr se2 #f))) ((##core#define-syntax) (let* ((var (cadr x)) @@ -616,22 +618,22 @@ name (##sys#current-environment) (##sys#eval/meta body)) - (compile '(##core#undefined) e #f tf cntr se) ) ) + (compile '(##core#undefined) e #f tf cntr se #f) ) ) ((##core#define-compiler-syntax) - (compile '(##core#undefined) e #f tf cntr se)) + (compile '(##core#undefined) e #f tf cntr se #f)) ((##core#let-compiler-syntax) (compile (##sys#canonicalize-body (cddr x) se #f) - e #f tf cntr se)) + e #f tf cntr se #f)) ((##core#include) (##sys#include-forms-from-file (cadr x) (caddr x) (lambda (forms) - (compile `(##core#begin ,@forms) e #f tf cntr se)))) + (compile `(##core#begin ,@forms) e #f tf cntr se tl?)))) ((##core#let-module-alias) (##sys#with-module-aliases @@ -640,7 +642,7 @@ (strip-syntax b)) (cadr x)) (lambda () - (compile `(##core#begin ,@(cddr x)) e #f tf cntr se)))) + (compile `(##core#begin ,@(cddr x)) e #f tf cntr se tl?)))) ((##core#module) (let* ((x (strip-syntax x)) @@ -691,14 +693,15 @@ (cons (compile (car body) '() #f tf cntr - (##sys#current-environment)) + (##sys#current-environment) + #t) ; reset back to toplevel! xs))))) ) ))) [(##core#loop-lambda) - (compile `(,(rename 'lambda se) ,@(cdr x)) e #f tf cntr se) ] + (compile `(,(rename 'lambda se) ,@(cdr x)) e #f tf cntr se #f) ] [(##core#provide) - (compile `(##sys#provide (##core#quote ,(cadr x))) e #f tf cntr se)] + (compile `(##sys#provide (##core#quote ,(cadr x))) e #f tf cntr se #f)] [(##core#require-for-syntax) (let ((id (cadr x))) @@ -708,30 +711,30 @@ ,@(map (lambda (x) `(##sys#load-extension (##core#quote ,x))) (lookup-runtime-requirements id))) - e #f tf cntr se))] + e #f tf cntr se #f))] [(##core#require) (let ((id (cadr x)) (alternates (cddr x))) (let-values (((exp _ _) (##sys#process-require id #f alternates))) - (compile exp e #f tf cntr se)))] + (compile exp e #f tf cntr se #f)))] [(##core#elaborationtimeonly ##core#elaborationtimetoo) ; <- Note this! (##sys#eval/meta (cadr x)) - (compile '(##core#undefined) e #f tf cntr se) ] + (compile '(##core#undefined) e #f tf cntr se tl?) ] [(##core#compiletimetoo) - (compile (cadr x) e #f tf cntr se) ] + (compile (cadr x) e #f tf cntr se tl?) ] [(##core#compiletimeonly ##core#callunit) - (compile '(##core#undefined) e #f tf cntr se) ] + (compile '(##core#undefined) e #f tf cntr se tl?) ] [(##core#declare) (##sys#notice "declarations are ignored in interpreted code" x) - (compile '(##core#undefined) e #f tf cntr se) ] + (compile '(##core#undefined) e #f tf cntr se #f) ] [(##core#define-inline ##core#define-constant) - (compile `(,(rename 'define se) ,@(cdr x)) e #f tf cntr se) ] + (compile `(,(rename 'define se) ,@(cdr x)) e #f tf cntr se #f) ] [(##core#primitive ##core#inline ##core#inline_allocate ##core#foreign-lambda ##core#define-foreign-variable @@ -744,13 +747,13 @@ (compile-call (cdr x) e tf cntr se) ] ((##core#the) - (compile (cadddr x) e h tf cntr se)) + (compile (cadddr x) e h tf cntr se tl?)) ((##core#typecase) ;; drops exp and requires "else" clause (cond ((assq 'else (strip-syntax (cdddr x))) => (lambda (cl) - (compile (cadr cl) e h tf cntr se))) + (compile (cadr cl) e h tf cntr se tl?))) (else (##sys#syntax-error-hook 'compiler-typecase @@ -789,7 +792,7 @@ (let* ((head (##sys#slot x 0)) (fn (if (procedure? head) (lambda _ head) - (compile (##sys#slot x 0) e #f tf cntr se))) + (compile (##sys#slot x 0) e #f tf cntr se #f))) (args (##sys#slot x 1)) (argc (checked-length args)) (info x) ) @@ -798,34 +801,34 @@ [(0) (lambda (v) (emit-trace-info tf info cntr e v) ((##core#app fn v)))] - [(1) (let ([a1 (compile (##sys#slot args 0) e #f tf cntr se)]) + [(1) (let ((a1 (compile (##sys#slot args 0) e #f tf cntr se #f))) (lambda (v) (emit-trace-info tf info cntr e v) ((##core#app fn v) (##core#app a1 v))) ) ] - [(2) (let* ([a1 (compile (##sys#slot args 0) e #f tf cntr se)] - [a2 (compile (##core#inline "C_u_i_list_ref" args 1) e #f tf cntr se)] ) + [(2) (let* ((a1 (compile (##sys#slot args 0) e #f tf cntr se #f)) + (a2 (compile (##core#inline "C_u_i_list_ref" args 1) e #f tf cntr se #f)) ) (lambda (v) (emit-trace-info tf info cntr e v) ((##core#app fn v) (##core#app a1 v) (##core#app a2 v))) ) ] - [(3) (let* ([a1 (compile (##sys#slot args 0) e #f tf cntr se)] - [a2 (compile (##core#inline "C_u_i_list_ref" args 1) e #f tf cntr se)] - [a3 (compile (##core#inline "C_u_i_list_ref" args 2) e #f tf cntr se)] ) + [(3) (let* ((a1 (compile (##sys#slot args 0) e #f tf cntr se #f)) + (a2 (compile (##core#inline "C_u_i_list_ref" args 1) e #f tf cntr se #f)) + (a3 (compile (##core#inline "C_u_i_list_ref" args 2) e #f tf cntr se #f)) ) (lambda (v) (emit-trace-info tf info cntr e v) ((##core#app fn v) (##core#app a1 v) (##core#app a2 v) (##core#app a3 v))) ) ] - [(4) (let* ([a1 (compile (##sys#slot args 0) e #f tf cntr se)] - [a2 (compile (##core#inline "C_u_i_list_ref" args 1) e #f tf cntr se)] - [a3 (compile (##core#inline "C_u_i_list_ref" args 2) e #f tf cntr se)] - [a4 (compile (##core#inline "C_u_i_list_ref" args 3) e #f tf cntr se)] ) + [(4) (let* ((a1 (compile (##sys#slot args 0) e #f tf cntr se #f)) + (a2 (compile (##core#inline "C_u_i_list_ref" args 1) e #f tf cntr se #f)) + (a3 (compile (##core#inline "C_u_i_list_ref" args 2) e #f tf cntr se #f)) + (a4 (compile (##core#inline "C_u_i_list_ref" args 3) e #f tf cntr se #f)) ) (lambda (v) (emit-trace-info tf info cntr e v) ((##core#app fn v) (##core#app a1 v) (##core#app a2 v) (##core#app a3 v) (##core#app a4 v))) ) ] - [else (let ([as (##sys#map (lambda (a) (compile a e #f tf cntr se)) args)]) + [else (let ((as (##sys#map (lambda (a) (compile a e #f tf cntr se #f)) args))) (lambda (v) (emit-trace-info tf info cntr e v) (apply (##core#app fn v) (##sys#map (lambda (a) (##core#app a v)) as))) ) ] ) ) ) - (compile exp env #f (fx> (##sys#eval-debug-level) 0) cntr se) ) ) ) + (compile exp env #f (fx> (##sys#eval-debug-level) 0) cntr se tl?) ) ) ) ;;; evaluate in the macro-expansion/compile-time environment @@ -846,8 +849,10 @@ ((compile-to-closure form '() - (##sys#current-meta-environment)) ;XXX evalenv? static? - '() ) ) + (##sys#current-meta-environment) + #f #f #f ;XXX evalenv? static? + #t) ; toplevel. + '()) ) (lambda () (##sys#active-eval-environment aee) (##sys#current-module oldcm) @@ -865,11 +870,11 @@ (let ((se2 (##sys#slot env 2))) ((if se2 ; not interaction-environment? (parameterize ((##sys#macro-environment '())) - (compile-to-closure x '() se2 #f env (##sys#slot env 3))) - (compile-to-closure x '() se #f env #f)) + (compile-to-closure x '() se2 #f env (##sys#slot env 3) #t)) + (compile-to-closure x '() se #f env #f #t)) '() ) ) ) (else - ((compile-to-closure x '() se #f #f #f) '()))))))) + ((compile-to-closure x '() se #f #f #f #t) '()))))))) (define (eval x . env) (apply (eval-handler) x env)) diff --git a/expand.scm b/expand.scm index c279512..9e194b9 100644 --- a/expand.scm +++ b/expand.scm @@ -1044,7 +1044,7 @@ (##sys#register-export name (##sys#current-module))) (when (c (r 'define) head) (chicken.expand#defjam-error x)) - `(##core#set! + `(##core#define-toplevel ,head ,(if (pair? body) (car body) '(##core#undefined))) ) ((pair? (car head)) diff --git a/tests/functor-tests.scm b/tests/functor-tests.scm index 173b8d4..8d109e6 100644 --- a/tests/functor-tests.scm +++ b/tests/functor-tests.scm @@ -166,6 +166,8 @@ (import chicken X) yibble) +;; XXX This is somewhat iffy: functor instantiation results in a +;; value! (test-equal "alternative functor instantiation syntax" (module yabble = frob (import scheme) (define yibble 99)) diff --git a/tests/syntax-tests.scm b/tests/syntax-tests.scm index a43b20e..4f07a3c 100644 --- a/tests/syntax-tests.scm +++ b/tests/syntax-tests.scm @@ -783,6 +783,13 @@ ) |# +;;; Definitions in expression contexts are rejected (#1309) + +(f (eval '(+ 1 2 (define x 3) 4))) +(f (eval '(display (define x 1)))) +;; Some tests for nested but valid definition expressions: +(t 2 (eval '(begin (define x 1) 2))) +(t 2 (eval '(module _ () (import scheme) (define x 1) 2))) ;;; renaming of keyword argument (#277) -- 2.1.4
From 0d7d60f502ebef3071bf65ee00fe507da6155a06 Mon Sep 17 00:00:00 2001 From: Peter Bex <pe...@more-magic.net> Date: Sat, 25 Feb 2017 15:45:41 +0100 Subject: [PATCH 2/5] Fix invalid definition caught by previous commit. This was inadvertantly treated as a global definition before! --- chicken-profile.scm | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/chicken-profile.scm b/chicken-profile.scm index 85120fa..4a6bbb8 100644 --- a/chicken-profile.scm +++ b/chicken-profile.scm @@ -138,14 +138,14 @@ EOF (set! sort-by sort-by-time) (define (set-decimals arg) + (define (arg-digit n) + (let ((n (- (char->integer (string-ref arg n)) + (char->integer #\0)))) + (if (<= 0 n 9) + (if (= n 9) 8 n) ; 9 => overflow in format-real + (error "invalid argument to -decimals option" arg)))) (if (= (string-length arg) 3) (begin - (define (arg-digit n) - (let ((n (- (char->integer (string-ref arg n)) - (char->integer #\0)))) - (if (<= 0 n 9) - (if (= n 9) 8 n) ; 9 => overflow in format-real - (error "invalid argument to -decimals option" arg)))) (set! seconds-digits (arg-digit 0)) (set! average-digits (arg-digit 1)) (set! percent-digits (arg-digit 2))) -- 2.1.4
From cfcaaa3a1bb97d09e3457623a53cf6b5162d1957 Mon Sep 17 00:00:00 2001 From: Peter Bex <pe...@more-magic.net> Date: Sat, 25 Feb 2017 17:04:28 +0100 Subject: [PATCH 3/5] Export internal define-like definitions from chicken.syntax Without this, the compiler would "inline" these aggressively as unspecified, because they're not assigned to from within the module itself. --- expand.scm | 20 +++++++++++++------- tests/syntax-tests.scm | 3 +++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/expand.scm b/expand.scm index 9e194b9..9ee0140 100644 --- a/expand.scm +++ b/expand.scm @@ -42,7 +42,13 @@ strip-syntax syntax-error er-macro-transformer - ir-macro-transformer) + ir-macro-transformer + + ;; These must be exported or the compiler will assume they're never + ;; assigned to. + define-definition + define-syntax-definition + define-values-definition) (import scheme chicken chicken.keyword) @@ -471,9 +477,9 @@ ; ; This code is disgustingly complex. -(define chicken.expand#define-definition) -(define chicken.expand#define-syntax-definition) -(define chicken.expand#define-values-definition) +(define define-definition) +(define define-syntax-definition) +(define define-values-definition) (define ##sys#canonicalize-body (lambda (body #!optional (se (##sys#current-environment)) cs?) @@ -481,9 +487,9 @@ (let ((f (lookup id se))) (or (eq? s f) (case s - ((define) (if f (eq? f chicken.expand#define-definition) (eq? s id))) - ((define-syntax) (if f (eq? f chicken.expand#define-syntax-definition) (eq? s id))) - ((define-values) (if f (eq? f chicken.expand#define-values-definition) (eq? s id))) + ((define) (if f (eq? f define-definition) (eq? s id))) + ((define-syntax) (if f (eq? f define-syntax-definition) (eq? s id))) + ((define-values) (if f (eq? f define-values-definition) (eq? s id))) (else (eq? s id)))))) (define (fini vars vals mvars body) (if (and (null? vars) (null? mvars)) diff --git a/tests/syntax-tests.scm b/tests/syntax-tests.scm index 4f07a3c..1da12c3 100644 --- a/tests/syntax-tests.scm +++ b/tests/syntax-tests.scm @@ -790,6 +790,9 @@ ;; Some tests for nested but valid definition expressions: (t 2 (eval '(begin (define x 1) 2))) (t 2 (eval '(module _ () (import scheme) (define x 1) 2))) +(t 1 (eval '(let () + (define-record-type foo (make-foo bar) foo? (bar foo-bar)) + (foo-bar (make-foo 1))))) ;;; renaming of keyword argument (#277) -- 2.1.4
From 7aa1fbaca16c4ecd0310b843290512ebabc21af5 Mon Sep 17 00:00:00 2001 From: Peter Bex <pe...@more-magic.net> Date: Sat, 25 Feb 2017 21:04:45 +0100 Subject: [PATCH 4/5] Change the way LET bodies are macro-expanded. A macro might expand into a define. That means we need to keep expanding the body and restart the main expansion process when we encounter a define. Instead of returning the original expressions when wrapping up, we should return the macro-expanded expressions, because macros should be called exactly once to be safe in the presence of side-effects. We now also treat ##core#begin as a reason to restart the expansion process, because nested begins can contain definitions as well. The expansion will recursively eliminate those nested begins. There is some special treatment for ##core#module and "import", because those do all kinds of nasty side-effecting things which ensure we can't simply expand the body in one go. Import is one of those aforementioned side-effecting macros, and the core module form is also side-effecting in a way: we can't refer to the module until it has been processed by the compiler. There could be other macros and special forms that are allowed in let bodies but need special processing. This situation needs to be addressed properly and fixed in general, but for now we can fix them by adding more special cases. Note that this is not a newly introduced problem: there have always been such issues, but due to the obscure workings of ##sys#canonicalize-body they would only surface under very specific conditions. --- expand.scm | 103 ++++++++++++++++++++++++++++++------------------- tests/syntax-tests.scm | 4 ++ 2 files changed, 68 insertions(+), 39 deletions(-) diff --git a/expand.scm b/expand.scm index 9ee0140..b1a91eb 100644 --- a/expand.scm +++ b/expand.scm @@ -480,6 +480,7 @@ (define define-definition) (define define-syntax-definition) (define define-values-definition) +(define import-definition) (define ##sys#canonicalize-body (lambda (body #!optional (se (##sys#current-environment)) cs?) @@ -490,24 +491,52 @@ ((define) (if f (eq? f define-definition) (eq? s id))) ((define-syntax) (if f (eq? f define-syntax-definition) (eq? s id))) ((define-values) (if f (eq? f define-values-definition) (eq? s id))) + ((import) (if f (eq? f import-definition) (eq? s id))) (else (eq? s id)))))) (define (fini vars vals mvars body) (if (and (null? vars) (null? mvars)) - (let loop ([body2 body] [exps '()]) - (if (not (pair? body2)) - (cons + ;; Macro-expand body, and restart when defines are found. + (let loop ((body body) (exps '())) + (if (not (pair? body)) + (cons '##core#begin - body) ; no more defines, otherwise we would have called `expand' - (let ((x (car body2))) - (if (and (pair? x) - (let ((d (car x))) - (and (symbol? d) - (or (comp 'define d) - (comp 'define-values d))))) - (cons - '##core#begin - (##sys#append (reverse exps) (list (expand body2)))) - (loop (cdr body2) (cons x exps)) ) ) ) ) + (reverse exps)) ; no more defines, otherwise we would have called `expand' + (let loop2 ((body body)) + (let ((x (car body)) + (rest (cdr body))) + (if (and (pair? x) + (let ((d (car x))) + (and (symbol? d) + (or (comp 'define d) + (comp 'define-values d) + (comp 'define-syntax d) + (comp '##core#begin d) + (comp 'import d))))) + ;; Stupid hack to avoid expanding imports + (if (comp 'import (car x)) + (loop rest (cons x exps)) + (cons + '##core#begin + (##sys#append (reverse exps) (list (expand body))))) + (let ((x2 (##sys#expand-0 x se cs?))) + (if (eq? x x2) + ;; Modules must be registered before we + ;; can continue with other forms, so + ;; hand back control to the compiler + (if (and (pair? x) + (symbol? (car x)) + (comp '##core#module (car x))) + `(##core#begin + ,@(reverse exps) + ,x + ,@(if (null? rest) + '() + `((##core#let () ,@rest)))) + (loop rest (cons x exps))) + (loop2 (cons x2 rest)) )) ))) )) + ;; We saw defines. Translate to letrec, and let compiler + ;; call us again for the remaining body by wrapping the + ;; remaining body forms in a ##core#let. (let* ((result `(##core#let ,(##sys#map @@ -549,6 +578,8 @@ (defjam-error def)) (loop (cdr body) (cons def defs) #f))) (else (loop body defs #t)))))) + ;; Expand a run of defines or define-syntaxes into letrec. As + ;; soon as we encounter something else, finish up. (define (expand body) ;; Each #t in "mvars" indicates an MV-capable "var". Non-MV ;; vars (#f in mvars) are 1-element lambda-lists for simplicity. @@ -598,14 +629,7 @@ (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 - (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))))))))))) + (else (fini vars vals mvars body)))))))) (expand body) ) ) @@ -959,23 +983,24 @@ ##sys#current-environment ##sys#macro-environment #f #t 'reexport))) -(##sys#extend-macro-environment - 'import '() - (##sys#er-transformer - (lambda (x r c) - `(##core#begin - ,@(map (lambda (x) - (let-values (((name lib spec v s i) (##sys#decompose-import x r c 'import))) - (if (not spec) - (##sys#syntax-error-hook - 'import "cannot import from undefined module" name) - (##sys#import - spec v s i - ##sys#current-environment ##sys#macro-environment #f #f 'import)) - (if (not lib) - '(##core#undefined) - `(##core#require ,lib ,(module-requirement name))))) - (cdr x)))))) +(set! chicken.expand#import-definition + (##sys#extend-macro-environment + 'import '() + (##sys#er-transformer + (lambda (x r c) + `(##core#begin + ,@(map (lambda (x) + (let-values (((name lib spec v s i) (##sys#decompose-import x r c 'import))) + (if (not spec) + (##sys#syntax-error-hook + 'import "cannot import from undefined module" name) + (##sys#import + spec v s i + ##sys#current-environment ##sys#macro-environment #f #f 'import)) + (if (not lib) + '(##core#undefined) + `(##core#require ,lib ,(module-requirement name))))) + (cdr x))))))) (##sys#extend-macro-environment 'begin-for-syntax '() diff --git a/tests/syntax-tests.scm b/tests/syntax-tests.scm index 1da12c3..6cbb751 100644 --- a/tests/syntax-tests.scm +++ b/tests/syntax-tests.scm @@ -794,6 +794,10 @@ (define-record-type foo (make-foo bar) foo? (bar foo-bar)) (foo-bar (make-foo 1))))) +;; Nested begins inside definitions were not treated correctly +(t 3 (eval '(let () (begin 1 (begin 2 (define internal-def 3) internal-def))))) +(f (eval '(let () internal-def))) + ;;; renaming of keyword argument (#277) (define-syntax foo1 -- 2.1.4
From eebe22419e3df08c9d7b5e4e7acd148c6dcc58c7 Mon Sep 17 00:00:00 2001 From: Peter Bex <pe...@more-magic.net> Date: Sat, 18 Mar 2017 14:15:35 +0100 Subject: [PATCH 5/5] Add expander hook so compiler can track line numbers. This restores (and even improves) precision of line number reporting in let bodies. Now that ##sys#canonicalize-body is performing macro expansion, we need a way for the compiler to update its line number database. This information got lost in the preceding commit. --- core.scm | 65 +++++++++++++++++++++++++++++++---------------- expand.scm | 7 ++--- tests/scrutiny-2.expected | 10 ++++---- 3 files changed, 52 insertions(+), 30 deletions(-) diff --git a/core.scm b/core.scm index b24e5ca..8fc8fc2 100644 --- a/core.scm +++ b/core.scm @@ -509,6 +509,18 @@ (##sys#put! alias '##core#macro-alias (lookup var se)) alias) ) + (define (handle-expansion-result outer-ln) + (lambda (input output) + (and-let* (((not (eq? input output))) + (ln (or (get-line input) outer-ln))) + (update-line-number-database! output ln)) + output)) + + (define (canonicalize-body/ln ln body se cs?) + (fluid-let ((expansion-result-hook + (handle-expansion-result ln))) + (##sys#canonicalize-body body se cs?))) + (define (set-real-names! as ns) (for-each (lambda (a n) (set-real-name! a n)) as ns) ) @@ -601,8 +613,10 @@ (set! ##sys#syntax-error-culprit x) (let* ((name0 (lookup (car x) se)) (name (or (and (symbol? name0) (##sys#get name0 '##core#primitive)) name0)) - (xexpanded (expand x se compiler-syntax-enabled))) - (when ln (update-line-number-database! xexpanded ln)) + (xexpanded + (fluid-let ((expansion-result-hook + (handle-expansion-result ln))) + (expand x se compiler-syntax-enabled)))) (cond ((not (eq? x xexpanded)) (walk xexpanded e se dest ldest h ln tl?)) @@ -690,14 +704,15 @@ (let* ((bindings (cadr x)) (vars (unzip1 bindings)) (aliases (map gensym vars)) - (se2 (##sys#extend-se se vars aliases))) + (se2 (##sys#extend-se se vars aliases)) + (ln (or (get-line x) outer-ln))) (set-real-names! aliases vars) `(let ,(map (lambda (alias b) (list alias (walk (cadr b) e se (car b) #t h ln #f)) ) aliases bindings) - ,(walk (##sys#canonicalize-body - (cddr x) se2 compiler-syntax-enabled) + ,(walk (canonicalize-body/ln + ln (cddr x) se2 compiler-syntax-enabled) (append aliases e) se2 dest ldest h ln #f) ) ) ) @@ -745,9 +760,10 @@ llist (lambda (vars argc rest) (let* ((aliases (map gensym vars)) + (ln (or (get-line x) outer-ln)) (se2 (##sys#extend-se se vars aliases)) - (body0 (##sys#canonicalize-body - obody se2 compiler-syntax-enabled)) + (body0 (canonicalize-body/ln + ln obody se2 compiler-syntax-enabled)) (body (walk (if emit-debug-info `(##core#begin @@ -787,11 +803,12 @@ (##sys#eval/meta (cadr b)) (strip-syntax (car b))))) (cadr x) ) - se) ) ) + se) ) + (ln (or (get-line x) outer-ln))) (walk - (##sys#canonicalize-body (cddr x) se2 compiler-syntax-enabled) - e se2 - dest ldest h ln #f) ) ) + (canonicalize-body/ln + ln (cddr x) se2 compiler-syntax-enabled) + e se2 dest ldest h ln #f) ) ) ((##core#letrec-syntax) (let* ((ms (map (lambda (b) @@ -802,13 +819,15 @@ (##sys#eval/meta (cadr b)) (strip-syntax (car b))))) (cadr x) ) ) - (se2 (append ms se)) ) + (se2 (append ms se)) + (ln (or (get-line x) outer-ln)) ) (for-each (lambda (sb) (set-car! (cdr sb) se2) ) ms) (walk - (##sys#canonicalize-body (cddr x) se2 compiler-syntax-enabled) + (canonicalize-body/ln + ln (cddr x) se2 compiler-syntax-enabled) e se2 dest ldest h ln #f))) ((##core#define-syntax) @@ -882,7 +901,8 @@ (strip-syntax (car b))) se)) (##sys#get name '##compiler#compiler-syntax) ) ) ) - (cadr x)))) + (cadr x))) + (ln (or (get-line x) outer-ln))) (dynamic-wind (lambda () (for-each @@ -891,8 +911,8 @@ bs) ) (lambda () (walk - (##sys#canonicalize-body - (cddr x) se compiler-syntax-enabled) + (canonicalize-body/ln + ln (cddr x) se compiler-syntax-enabled) e se dest ldest h ln tl?) ) (lambda () (for-each @@ -1010,15 +1030,16 @@ body)))) ((##core#loop-lambda) ;XXX is this really needed? - (let* ([vars (cadr x)] - [obody (cddr x)] - [aliases (map gensym vars)] + (let* ((vars (cadr x)) + (obody (cddr x)) + (aliases (map gensym vars)) (se2 (##sys#extend-se se vars aliases)) - [body + (ln (or (get-line x) outer-ln)) + (body (walk - (##sys#canonicalize-body obody se2 compiler-syntax-enabled) + (canonicalize-body/ln ln obody se2 compiler-syntax-enabled) (append aliases e) - se2 #f #f dest ln #f) ] ) + se2 #f #f dest ln #f) ) ) (set-real-names! aliases vars) `(##core#lambda ,aliases ,body) ) ) diff --git a/expand.scm b/expand.scm index b1a91eb..d1d8ee3 100644 --- a/expand.scm +++ b/expand.scm @@ -48,7 +48,8 @@ ;; assigned to. define-definition define-syntax-definition - define-values-definition) + define-values-definition + expansion-result-hook) (import scheme chicken chicken.keyword) @@ -259,7 +260,7 @@ "' returns original form, which would result in endless expansion") exp)) (dx `(,name --> ,exp2)) - exp2))) + (expansion-result-hook exp exp2) ) ) ) (define (expand head exp mdef) (dd `(EXPAND: ,head @@ -316,7 +317,7 @@ (define ##sys#compiler-syntax-hook #f) (define ##sys#enable-runtime-macros #f) - +(define expansion-result-hook (lambda (input output) output)) ;;; User-level macroexpansion diff --git a/tests/scrutiny-2.expected b/tests/scrutiny-2.expected index 4cabcc4..412e7a5 100644 --- a/tests/scrutiny-2.expected +++ b/tests/scrutiny-2.expected @@ -1,18 +1,18 @@ Note: at toplevel: - (scrutiny-tests-2.scm:14) in procedure call to `pair?', the predicate is called with an argument of type `pair' and will always return true + (scrutiny-tests-2.scm:20) in procedure call to `pair?', the predicate is called with an argument of type `pair' and will always return true Note: at toplevel: - (scrutiny-tests-2.scm:14) in procedure call to `pair?', the predicate is called with an argument of type `null' and will always return false + (scrutiny-tests-2.scm:20) in procedure call to `pair?', the predicate is called with an argument of type `null' and will always return false Note: at toplevel: - (scrutiny-tests-2.scm:14) in procedure call to `pair?', the predicate is called with an argument of type `null' and will always return false + (scrutiny-tests-2.scm:20) in procedure call to `pair?', the predicate is called with an argument of type `null' and will always return false Note: at toplevel: - (scrutiny-tests-2.scm:14) in procedure call to `pair?', the predicate is called with an argument of type `fixnum' and will always return false + (scrutiny-tests-2.scm:20) in procedure call to `pair?', the predicate is called with an argument of type `fixnum' and will always return false Note: at toplevel: - (scrutiny-tests-2.scm:14) in procedure call to `pair?', the predicate is called with an argument of type `float' and will always return false + (scrutiny-tests-2.scm:20) in procedure call to `pair?', the predicate is called with an argument of type `float' and will always return false Note: at toplevel: (scrutiny-tests-2.scm:21) in procedure call to `list?', the predicate is called with an argument of type `null' and will always return true -- 2.1.4
signature.asc
Description: Digital signature
_______________________________________________ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers