Re: [Chicken-hackers] [PATCH] Improve hygiene of FFI macros
Peter Bex wrote: > Attached is an updated version of my initial patch but without that one > patch hunk that broke canonicalize-expression. This looks good, I’ve applied it. ___ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers
Re: [Chicken-hackers] [PATCH] Improve hygiene of FFI macros
On Tue, Apr 09, 2019 at 12:48:16PM +0200, Peter Bex wrote: > The patch is somewhat large but straightforward. It replaces all uses of > just "quote" (via the single quote character) to "##core#quote" Hi all, This patch contains one bug; in canonicalize-expression, I replaced a quote by ##core#quote which wasn't walked, which means it would end up in the canonicalized language as understood by perform-cps-conversion, which only has plain unqualified "quote" (but that's *after* macro-expansion, so it is unrelated to "quote" from the "scheme" module). This is only triggered in unsafe code, so when recompiling CHICKEN with itself it would fail on scheduler.scm. Attached is an updated version of my initial patch but without that one patch hunk that broke canonicalize-expression. Cheers, Peter From 5be780755871372eccddf63f19b5ea240118c6b4 Mon Sep 17 00:00:00 2001 From: Peter Bex Date: Tue, 9 Apr 2019 12:28:15 +0200 Subject: [PATCH] Improve hygiene of FFI macros These macros would expand to renamed identifiers but those identifiers were not in the syntactic environment of these macros, so they would "escape" and require the user to import "scheme" and possible "chicken.base" without prefixes to have them work. Also, the helper procedures used single quotes in the code they generated, which expands to unqualified "quote". Replace these by ##core#quote. --- NEWS | 3 ++ chicken-ffi-syntax.scm | 18 core.scm | 36 +++ support.scm | 113 --- tests/compiler-tests.scm | 4 +- 5 files changed, 90 insertions(+), 84 deletions(-) diff --git a/NEWS b/NEWS index c8f21f8b..ad83d6ba 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,9 @@ the compiler when later trying to import that same module (fixes #1506, thanks to Kristian Lein-Mathisen). +- Foreign function interface + - Improved hygiene in FFI macro expansions, which means you don't + have to import "scheme" or "(chicken base)" for them to work. 5.0.1 diff --git a/chicken-ffi-syntax.scm b/chicken-ffi-syntax.scm index f0edba43..da7a6fc6 100644 --- a/chicken-ffi-syntax.scm +++ b/chicken-ffi-syntax.scm @@ -63,7 +63,9 @@ (##sys#extend-macro-environment 'define-external - '() + `((define . ,(alist-ref 'define me0)) ; Or just me0? + (begin . ,(alist-ref 'begin me0)) + (lambda . ,(alist-ref 'lambda me0))) (compiler-only-er-transformer (lambda (form r c) (let* ((form (cdr form)) @@ -82,14 +84,14 @@ (if quals (##sys#check-syntax 'define-external form '(string (symbol . #((_ symbol) 0)) _ . #(_ 1))) (##sys#check-syntax 'define-external form '((symbol . #((_ symbol) 0)) _ . #(_ 1))) ) - (let* ([head (if quals (cadr form) (car form))] - [args (cdr head)] ) + (let* ((head (if quals (cadr form) (car form))) + (args (cdr head)) ) `(,(r 'define) ,(car head) (##core#foreign-callback-wrapper - ',(car head) + (##core#quote ,(car head)) ,(if quals (car form) "") - ',(if quals (caddr form) (cadr form)) - ',(map (lambda (a) (car a)) args) + (##core#quote ,(if quals (caddr form) (cadr form))) + (##core#quote ,(map (lambda (a) (car a)) args)) (,(r 'lambda) ,(map (lambda (a) (cadr a)) args) ,@(if quals (cdddr form) (cddr form)) ) ) ) ) ] ) ) ) ) ) @@ -108,7 +110,7 @@ (##sys#extend-macro-environment 'define-location - '() + `((begin . ,(alist-ref 'begin me0))) (compiler-only-er-transformer (lambda (form r c) (##sys#check-syntax 'define-location form '(_ variable _ . #(_ 0 1))) @@ -161,7 +163,7 @@ (##sys#extend-macro-environment 'foreign-code - '() + `((declare . ,(alist-ref 'declare me0))) (compiler-only-er-transformer (lambda (form r c) (##sys#check-syntax 'foreign-code form '(_ . #(string 0))) diff --git a/core.scm b/core.scm index 06a4cf7f..7b68a063 100644 --- a/core.scm +++ b/core.scm @@ -660,7 +660,7 @@ ((##core#check) (if unsafe - ''#t + '(quote #t) (walk (cadr x) e dest ldest h ln tl?) ) ) ((##core#the) @@ -793,7 +793,7 @@ (walk (if emit-debug-info `(##core#begin - (##core#debug-event C_DEBUG_ENTRY ',dest) + (##core#debug-event C_DEBUG_ENTRY (##core#quote ,dest)) ,body0) body0) (append aliases e) #f #f dest ln #f @@ -879,7 +879,7 @@ (walk (if ##sys#enable-runtime-macros `(##sys#extend-macro-environment - ',var + (##core#quote ,var) (##sys#current-environment) ,body) ;XXX possibly wrong se? '(##core#undefined) ) e dest ldest h ln #f)) ) @@ -911,7 +911,7 @@ `(##sys#cons (##sys#ensure-transformer ,body - ',var) + (##core#quote ,var)) (##sys#current-environment '(##core#undefined) ) e dest ldest h ln #f))) @@ -1102,7 +1102,7 @@ (let ((type (second fv))
[Chicken-hackers] [PATCH] Improve hygiene of FFI macros
Hi all, I had a quick look at #1346 (which I haven't fixed yet) but then quickly noticed that define-external expands into "define" in an unhygienic way. You can notice this if you have a module with an import of ONLY (chicken foreign) but not scheme. Of course this is not a very common thing to do, which is why we never ran into it. I've modified one test to trigger an error with the current version, and fixed several issues with it. The fix is probably not 100% complete (and the test certainly isn't), but it's certainly an improvement. The patch is somewhat large but straightforward. It replaces all uses of just "quote" (via the single quote character) to "##core#quote", uses of plain "let" to ##core#let, and it adds some missing macros to the syntax environment so that the macros can expand properly to the intended macro calls. I'm not sure if we should pick just the used definitions from the macro environment (which is what I did here with alist-ref), or if we should just take the entire original macro environment (which would be more convenient and less error-prone when we change any macros to make use of other, new macros but also slightly slower(?)). Cheers, Peter From 07528942dd064843db9a55f56090b9507b9cc981 Mon Sep 17 00:00:00 2001 From: Peter Bex Date: Tue, 9 Apr 2019 12:28:15 +0200 Subject: [PATCH] Improve hygiene of FFI macros These macros would expand to renamed identifiers but those identifiers were not in the syntactic environment of these macros, so they would "escape" and require the user to import "scheme" and possible "chicken.base" without prefixes to have them work. Also, the helper procedures used single quotes in the code they generated, which expands to unqualified "quote". Replace these by ##core#quote. --- NEWS | 3 ++ chicken-ffi-syntax.scm | 18 core.scm | 36 +++ support.scm | 113 --- tests/compiler-tests.scm | 4 +- 5 files changed, 90 insertions(+), 84 deletions(-) diff --git a/NEWS b/NEWS index c8f21f8b..ad83d6ba 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,9 @@ the compiler when later trying to import that same module (fixes #1506, thanks to Kristian Lein-Mathisen). +- Foreign function interface + - Improved hygiene in FFI macro expansions, which means you don't + have to import "scheme" or "(chicken base)" for them to work. 5.0.1 diff --git a/chicken-ffi-syntax.scm b/chicken-ffi-syntax.scm index f0edba43..da7a6fc6 100644 --- a/chicken-ffi-syntax.scm +++ b/chicken-ffi-syntax.scm @@ -63,7 +63,9 @@ (##sys#extend-macro-environment 'define-external - '() + `((define . ,(alist-ref 'define me0)) ; Or just me0? + (begin . ,(alist-ref 'begin me0)) + (lambda . ,(alist-ref 'lambda me0))) (compiler-only-er-transformer (lambda (form r c) (let* ((form (cdr form)) @@ -82,14 +84,14 @@ (if quals (##sys#check-syntax 'define-external form '(string (symbol . #((_ symbol) 0)) _ . #(_ 1))) (##sys#check-syntax 'define-external form '((symbol . #((_ symbol) 0)) _ . #(_ 1))) ) - (let* ([head (if quals (cadr form) (car form))] - [args (cdr head)] ) + (let* ((head (if quals (cadr form) (car form))) + (args (cdr head)) ) `(,(r 'define) ,(car head) (##core#foreign-callback-wrapper - ',(car head) + (##core#quote ,(car head)) ,(if quals (car form) "") - ',(if quals (caddr form) (cadr form)) - ',(map (lambda (a) (car a)) args) + (##core#quote ,(if quals (caddr form) (cadr form))) + (##core#quote ,(map (lambda (a) (car a)) args)) (,(r 'lambda) ,(map (lambda (a) (cadr a)) args) ,@(if quals (cdddr form) (cddr form)) ) ) ) ) ] ) ) ) ) ) @@ -108,7 +110,7 @@ (##sys#extend-macro-environment 'define-location - '() + `((begin . ,(alist-ref 'begin me0))) (compiler-only-er-transformer (lambda (form r c) (##sys#check-syntax 'define-location form '(_ variable _ . #(_ 0 1))) @@ -161,7 +163,7 @@ (##sys#extend-macro-environment 'foreign-code - '() + `((declare . ,(alist-ref 'declare me0))) (compiler-only-er-transformer (lambda (form r c) (##sys#check-syntax 'foreign-code form '(_ . #(string 0))) diff --git a/core.scm b/core.scm index 06a4cf7f..cc440eb4 100644 --- a/core.scm +++ b/core.scm @@ -660,7 +660,7 @@ ((##core#check) (if unsafe - ''#t + '(##core#quote #t) (walk (cadr x) e dest ldest h ln tl?) ) ) ((##core#the) @@ -793,7 +793,7 @@ (walk (if emit-debug-info `(##core#begin - (##core#debug-event C_DEBUG_ENTRY ',dest) + (##core#debug-event C_DEBUG_ENTRY (##core#quote ,dest)) ,body0) body0) (append aliases e) #f #f dest ln #f @@ -879,7 +879,7 @@ (walk (if ##sys#enable-runtime-macros `(##sys#extend-macro-environment - ',var + (##core#quote ,var)
