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

[Tino Calancha <tino.calancha@gmail.com>]

Hi Alex,

thank you for your report and your time!
I answer you below.

>> 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
Eli means here "order on the bindings assignment is unspecified in a 'let'".
Programs shouldn't rely on any particular order in the assigments: it's an
implementation detail.

>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.
There is a good reason: the result code implementing cl-letf is simpler.
Your patch unnecessarily adds calls to `setq' and `nreverse'.  That result
in longer code and less efficient.
Bear in mind that `let' and `cl-letf' are written in different languages;
the former in C, the latter in elisp.  As far as those implementations
satisfy their specification, the simpler and more efficient that they
can be the better.

>> The value forms are evaluated in order, the bindings are not necessarily
>> in order.
>>
>> (let ((x 0))
>>   (cl-letf ((a (setq x 1))
>>             (a (setq x 2)))
>>     (list x a))) ;=> (2 1)
>
>Right, this expands to:
>
>(let ((x 0))
>  (let*
>      ((vnew
>        (setq x 1))
>       (vnew
>        (setq x 2))
>       (a vnew)
>       (a vnew))
>    (unwind-protect
>        (list x a))))
>
>Which, outside of the case of repeating the variable name (which
>arguably shouldn't be allowed like in some other Lisps), doesn't matter.
>
>It only matters when using more complex places like
>
>(cl-letf (((aref v 1) 10)
>          ((aref w 2) 20))
>  (aref v 1))
No it doesn't matter, because as pointed out above the order to perform the
parallel bindings is unspecified.  It might start binding from left to right,
or right to left, or even random order using the current time as a seed.
Code relying in a particular order for those bindings is not portable, even more,
it has a bug; we might change the implemention in the future for whatever
reason producing a different order: then such code will break.

If you want the bindings being perform from left to right, then you just need
to use sequential `letf*'/`cl-letf*' instead of the parallel `let'/`cl-letf'.

>Just the (nreverse simplebinds) line, which I added just to make cl-letf
>a little bit saner (i.e. more like let). This part does seem to be
>unspecified, but I don't see why it should unnecessarily diverge from
>let.
In addition to result in a simpler implementation, it's useful as a reminder
that code shouldn't assume a particular order.

Even if i don't see any problem with `cl-letf' implementation, i agree with
Philipp and Eli that it would be worth if the outcome of this report is an improvement
in the documentation.  We might update the manual adding a more precise statement
to clarify that the order to perform the parallel bindings is unspecified, i.e.,
code should not assume a particular order.
As Eli said patches are welcome.
Alex, are you willing to prepare those doc patches?

Thank you very much,
Tino