[PATCH] Use closures in hashcash.el

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Use closures in hashcash.el

Christopher Wellons
hashcash.el was switched over to lexical scope in da94ea9, but it still
uses an old workaround of dynamically constructing lambda expression
"closures" using backquote. The following patch changes these into
proper closures.

I discovered this because, in the first case, the backquote lambda is
actually constructed incorrectly. The callback function object is
evaluated an extra time when the callback is invoked. That's harmless
for non-closures and byte-compiled functions, but it doesn't work with
interpreted closures. I've made this same error myself and have written
about it here:

Emacs Lisp Lambda Expressions Are Not Self-Evaluating
https://nullprogram.com/blog/2018/02/22/

diff --git a/lisp/mail/hashcash.el b/lisp/mail/hashcash.el
index 59200eaab8..77b4a429d2 100644
--- a/lisp/mail/hashcash.el
+++ b/lisp/mail/hashcash.el
@@ -182,8 +182,8 @@ Return immediately.  Call CALLBACK with process and result when ready."
  (setq hashcash-process-alist (cons
       (cons process (current-buffer))
       hashcash-process-alist))
- (set-process-filter process `(lambda (process output)
-       (funcall ,callback process output))))
+ (set-process-filter process (lambda (process output)
+      (funcall callback process output))))
     (funcall callback nil nil)))
 
 (defun hashcash-check-payment (token str val)
@@ -244,8 +244,8 @@ Only start calculation.  Results are inserted when ready."
     (hashcash-generate-payment-async
      (hashcash-payment-to arg)
      (hashcash-payment-required arg)
-     `(lambda (process payment)
- (hashcash-insert-payment-async-2 ,(current-buffer) process payment)))))
+     (lambda (process payment)
+       (hashcash-insert-payment-async-2 (current-buffer) process payment)))))
 
 (defun hashcash-insert-payment-async-2 (buffer process pay)
   (when (buffer-live-p buffer)

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Use closures in hashcash.el

Stefan Monnier
> hashcash.el was switched over to lexical scope in da94ea9, but it still
> uses an old workaround of dynamically constructing lambda expression
> "closures" using backquote. The following patch changes these into
> proper closures.

Thanks, installed with the change below:

> - (set-process-filter process `(lambda (process output)
> -       (funcall ,callback process output))))
> + (set-process-filter process (lambda (process output)
> +      (funcall callback process output))))

Actually, this lambda expression is just a roundabout way to say
`callback` (by η-reduction).


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Use closures in hashcash.el

Christopher Wellons
Michael Heerdegen has pointed out that the second backtick change isn't
correct since it doesn't remember the original buffer. Here's a
correction (sorry!):

diff --git a/lisp/mail/hashcash.el b/lisp/mail/hashcash.el
index 519c6d94e1..6ee1aee55a 100644
--- a/lisp/mail/hashcash.el
+++ b/lisp/mail/hashcash.el
@@ -243,8 +243,9 @@ Only start calculation.  Results are inserted when ready."
     (hashcash-generate-payment-async
      (hashcash-payment-to arg)
      (hashcash-payment-required arg)
-     (lambda (process payment)
-       (hashcash-insert-payment-async-2 (current-buffer) process payment)))))
+     (let ((buffer (current-buffer)))
+       (lambda (process payment)
+         (hashcash-insert-payment-async-2 buffer process payment))))))
 
 (defun hashcash-insert-payment-async-2 (buffer process pay)
   (when (buffer-live-p buffer)

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Use closures in hashcash.el

John Shahid
In reply to this post by Stefan Monnier

Stefan Monnier <[hidden email]> writes:

[...]

> Thanks, installed with the change below:
>
>> - (set-process-filter process `(lambda (process output)
>> -       (funcall ,callback process output))))
>> + (set-process-filter process (lambda (process output)
>> +      (funcall callback process output))))

I think you meant:

- (set-process-filter process `(lambda (process output)
-       (funcall ,callback process output))))
+ (set-process-filter process callback))

which is what you indeed installed on master.