bug#25154: 25.1; Bindings in cl-letf are in reverse order

[Alex <agrambot@gmail.com>]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Date: Fri, 09 Dec 2016 17:36:15 -0600
>> 
>> Compare the following:
>> 
>> (let ((x 5)
>>       (x 6))
>>   (+ x 10))
>> 
>> => 16
>> 
>> (cl-letf ((x 5)
>>           (x 6))
>>   (+ x 10))
>> 
>> => 15
>
> Isn't it true that the order of evaluation in a 'let' is unspecified?
> If you want a particular order, use 'let*'.

I don't think so. See (info "(elisp) Local Variables"):

  All of the VALUE-FORMs in BINDINGS are evaluated in the order they
  appear

I believe it should follow for cl-letf. Besides, even if it was
unspecified, evaluating in the order they appear would be adhering to
the principle of least astonishment.

Though I realize that I have made a mistake in naming this bug and
patch. The bigger issue is that all (PLACE VALUE) pairs of each type
(simple and more importantly complex) were being evaluated in reverse
order. Take for example:

(cl-letf (((aref v 1) 10)
          ((aref w 2) 20))
  (aref v 1))


This approximately expands to:

(let*
    ((v v)
     (v w)
     (old
      (aref v 2))
     (old
      (aref v 1)))
  (unwind-protect
      (progn
        (aset v 2 20)
        (aset v 1 10)
        (aref v 1))
    (aset v 2 old)
    (aset v 1 old)))

As you can see, the arefs and asets are evaluated in reverse order.
Again, even if you argue that the order of evaluation for (PLACE VALUE)
pairs is unspecified, it's evaluating them in an unexpected way for no
good reason.

I have attached a reworded patch that expands the above into the more
expected:

(let*
    ((v v)
     (v w)
     (old
      (aref v 1))
     (old
      (aref v 2)))
  (unwind-protect
      (progn
        (aset v 1 10)
        (aset v 2 20)
        (aref v 1))
    (aset v 1 old)
    (aset v 2 old)))



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: cl-letf patch reworded --]
[-- Type: text/x-diff, Size: 1251 bytes --]

From 184fa8dcb738923994d4b287931849b182bdb522 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Fri, 9 Dec 2016 22:10:54 -0600
Subject: [PATCH] Preserve the order of (PLACE VALUE) pairs in cl-letf

Do not reverse the order of (PLACE VALUE) pairs in cl-letf (Bug #25154).

* lisp/emacs-lisp/cl-macs.el (cl--letf): Use nreverse to restore the
  order of (PLACE VALUE) pairs.
---
 lisp/emacs-lisp/cl-macs.el | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 210a208..34f124f 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -2388,7 +2388,10 @@ cl--letf
   ;;    VAL1; VAL2; PLACE1; bind1; PLACE2; bind2
   ;; Common-Lisp's `psetf' does the first, so we'll do the same.
   (if (null bindings)
-      (if (and (null binds) (null simplebinds)) (macroexp-progn body)
+      (if (and (null binds) (null simplebinds))
+          (macroexp-progn body)
+        (setq binds (nreverse binds))
+        (setq simplebinds (nreverse simplebinds))
         `(let* (,@(mapcar (lambda (x)
                             (pcase-let ((`(,vold ,getter ,_setter ,_vnew) x))
                               (list vold getter)))
-- 
2.10.2