[PATCH] lisp/ob-scheme.el

[Zelphir Kaltstahl <zelphirkaltstahl@posteo.de>]

[-- Attachment #1: Type: text/plain, Size: 2460 bytes --]

On 3/11/23 10:58, Ihor Radchenko wrote:
> Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:
>
>> The issue is not with defining via (define ...) inside a (let ...) in Guile. It
>> is about importing macros at the time, when the body of the (let ...) is already
>> evaluated, which is at a later phase than macro expansion. By wrapping inside a
>> (let ...) org has moved the import to a later phase, which causes the macro
>> (let-values ...) to not be expanded.
> I see.
> AFAIK, Elisp does not have this problem.
>
>> As far as I know, (defun ...) and (defvar ...) are merely defining functions and
>> variables, not macros.
> Same for defmacro in Elisp.
>
>> My point is, that imports are usually global for sessions. But :var decided for
>> let-wrapping, moving them to a different place. Just like imports are usually
>> global, I would expect (define ...)s to be global in the session, unless I put
>> them inside a narrowed scope like a (let ...) myself. The org generated (let
>> ...) is invisible to the user and thus confusing, at least for GNU Guile.
>>
>> For other Schemes it probably all depends on how their phases of expansion and
>> evaluation work. I don't know enough about the Scheme standards, to tell,
>> whether Guile has the correct behavior here or whether there is a correct
>> behavior defined in the Scheme standards. Maybe someone more knowledgeable can
>> chime in to comment on that.
> When saying Guile I mean scheme. Remember that I am now looking from a
> more general perspective of other ob-* libraries.
>
> My conclusion so far is that it is not safe in ob-scheme to use
> let-binding. Other ob-* lisp implementations may be OK (at least,
> ob-emacs-lisp is OK).
>
> Now, the main question is whether it is safe to use `define' in all the
> scheme implementations. If it is, would you be interested in turning
> your personal fix into a patch for ob-scheme?

Hi!

I've created a patch, which I will attach to this e-mail.

Not sure it meets all formalities. For example it is not clear to me, whether I 
should add the "TINYCHANGE" at the bottom of my commit message.

Still need to get around to test at least some other Scheme as well, but I guess 
I should get started with the patch, otherwise I will procrastinate or be stuck 
in fear of formalities forever.

Let me know, if this an OK patch or what else needs to be done or what format is 
wrong, if any.

-- 
repositories: https://notabug.org/ZelphirKaltstahl

[-- Attachment #2: 0001-lisp-ob-scheme.el.patch --]
[-- Type: text/x-patch, Size: 2204 bytes --]

From 51b299aa18e882681dd681acb51c9cb1b44f3b4e Mon Sep 17 00:00:00 2001
From: Zelphir Kaltstahl <zelphirkaltstahl@posteo.de>
Date: Sat, 18 Mar 2023 16:06:05 +0100
Subject: [PATCH] lisp/ob-scheme.el:

* ob-scheme.el (org-babel-expand-body:scheme,
org-babel-expand-header-arg-vars:scheme): Change the way header
argument :var variables are expanded for for Scheme source blocks.  Use
`define' instead of wrapping using `let'.

Wrapping binding definitions using `let' can lead to issues with GNU
Guile and potentially other Scheme dialects. GNU Guile will only get
to the body of the let at evaluation time, not at macro expansion
time. If the let form wraps any imports of libraries that define
macros, then those imported macros are seen too late and their
corresponding forms inside the body of the let are not
expanded. Using `define' to define bindings avoids this problem, at
least in GNU Guile.
---
 lisp/ob-scheme.el | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/lisp/ob-scheme.el b/lisp/ob-scheme.el
index 9f12f42cb..f837dedd2 100644
--- a/lisp/ob-scheme.el
+++ b/lisp/ob-scheme.el
@@ -79,6 +79,14 @@
 (defvar org-babel-default-header-args:scheme '()
   "Default header arguments for scheme code blocks.")
 
+(defun org-babel-expand-header-arg-vars:scheme (vars)
+  "Expand :var header arguments given as VARS."
+  (mapconcat
+   (lambda (var)
+     (format "(define %s %S)" (car var) (cdr var)))
+   vars
+   "\n"))
+
 (defun org-babel-expand-body:scheme (body params)
   "Expand BODY according to PARAMS, return the expanded body."
   (let ((vars (org-babel--get-vars params))
@@ -86,16 +94,9 @@
 	(postpends (cdr (assq :epilogue params))))
     (concat (and prepends (concat prepends "\n"))
 	    (if (null vars) body
-	      (format "(let (%s)\n%s\n)"
-		      (mapconcat
-		       (lambda (var)
-			 (format "%S" (print `(,(car var) ',(cdr var)))))
-		       vars
-		       "\n      ")
-		      body))
+	      (concat (org-babel-expand-header-arg-vars:scheme vars) body))
 	    (and postpends (concat "\n" postpends)))))
 
-
 (defvar org-babel-scheme-repl-map (make-hash-table :test #'equal)
   "Map of scheme sessions to session names.")
 
-- 
2.25.1