[PATCH] gv.el: minor change, cleaning up

[PATCH] gv.el: minor change, cleaning up

[Hu Lucius]

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

When defining `gv-expander` of `alist-get`, the following binding
specification seems to have a redundant `setq` form.

 ((set-exp
   `(if ,p (setcdr ,p ,v)
      ,(funcall setter
                `(cons (setq ,p (cons ,k ,v))
                       ,getter)))))

Here `set-exp` expands to an s-expression that updates the value
of an alist. `p` is a boolean that equals `(assq KEY ALIST)`
(or any other test function specified in the `alist-get` form).
`k` and `v` are KEY and VALUE respectively.

Evaluate `(macroexpand-all `(setf (alist-get key alist) value))` and
it returns the following:

  (let* () ;; /* bindings omitted */
    (progn
     (if p
         (setcdr p v)
       (setq alist
             (cons (setq p
                         (cons key v))
                   alist)))
     v))

It seems to me that `(setq p ..)` is unnecessary, i.e. we can simply
use

 (setq alist (cons (cons key v) alist)

This commit fixes that.

Signed-off-by: Lucius Hu <lebensterben@users.noreply.github.com>
---
 lisp/emacs-lisp/gv.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
index f08f7ac115..cf6cae4fd8 100644
--- a/lisp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -407,7 +407,7 @@ alist-get
                        (let ((set-exp
                               `(if ,p (setcdr ,p ,v)
                                  ,(funcall setter
-                                           `(cons (setq ,p (cons ,k ,v))
+                                           `(cons (cons ,k ,v)
                                                   ,getter)))))
                          `(progn
                             ,(cond
-- 
2.32.0

[-- Attachment #2: Type: text/html, Size: 2168 bytes --]

Re: [PATCH] gv.el: minor change, cleaning up

[Stefan Monnier]

> Evaluate `(macroexpand-all `(setf (alist-get key alist) value))` and
> it returns the following:
>
>   (let* () ;; /* bindings omitted */
>     (progn
>      (if p
>          (setcdr p v)
>        (setq alist
>              (cons (setq p
>                          (cons key v))
>                    alist)))
>      v))
>
> It seems to me that `(setq p ..)` is unnecessary,

It's unnecessary for `setf` (and `push`, and `cl-pushnew`, and
`cl-rotatef`, and `cl-callf` ;-) but it can be necessary for more
complex settings.  I can't remember what was the setting where I found
that I needed it, sadly.  Clearly it must be one where we use the getter
(or the setter) after having used the setter.


        Stefan