bug#67116: byte-compile-let: reversing the order of evaluation of the clauses CAN make a difference.

[Alan Mackenzie <acm@muc.de>]

Hello, Stefan.

On Sat, Nov 11, 2023 at 23:52:38 -0500, Stefan Monnier wrote:
> > In lisp/emacs-lisp/bytecomp.el (byte-compile-let), when the following
> > form (from jit-lock--debug-fontify):

> >                           (let
> >                               ((beg pos)
> >                                 (end (setq pos
> >                                                (next-single-property-change
> >                                                 pos 'fontified
> >                                                 nil (point-max)))))
> >                             (put-text-property beg end 'fontified nil)
                                (message "jit-lock-fontify-now %s %s" beg end)
> >                             (jit-lock-fontify-now beg end))

> > gets byte compiled, the order of evaluating BEG and END gets reversed so
> > that END gets evaluated first.

> Sounds like a bug.  Do you have some recipe to reproduce it?

Certainly!  In the code fragment above in jit-lock--debug-fontify,
insert the diagnostic line as above.  Byte compile this function.  Then
M-x jit-lock-debug-mode.  Now scrolling any (previously unfontified)
sections of a buffer will fail to fontify those sections.  In *Messages*
it can be seen that the printed values of BEG and END are identical,
hence the call to jit-lock-fontify-now does nothing.

> I looked at the bytecode but it's a bit hard to tell what's going on
> there, since the var names are lost along the way.

> > The comment in byte-compile-let:

> >       ;; Bind the variables.
> >       ;; For `let', do it in reverse order, because it makes no
> >       ;; semantic difference, but it is a lot more efficient since the
> >       ;; values are now in reverse order on the stack.

> > , is not true.  It can make a semantic difference.  So doing the binding
> > in reverse order is a bug.

> Note that this is talking about the actual binding operations, which is
> separate from the computation of the values that are to be given.
> What this is saying is that

>     (let ((X1 E1)
>           (X2 E2))
>       ...)

> can be compiled to

>     <compute E1>
>     <compute E2>
>     varbind X2
>     varbind X1

> since computing E pushes it value on the stack, so after the two
> "compute" we have the values of E1 and E2 on the stack and we can pop
> them in reverse order.

It seems apparent that the computations are being done in reverse order,
too.  That can be seen in the above *Messages* output as well as in
byte-compile-let in bytecomp.el:

      (dolist (var (if is-let (reverse clauses) clauses))
        (unless is-let
          (push (byte-compile-push-binding-init var) init-lexenv))
        (let ((var (if (consp var) (car var) var)))
          (if (byte-compile-bind var init-lexenv)
              (pop init-lexenv))))

..  (reverse clauses) happens before the code for any of the value forms
gets generated.

> And indeed it should make no difference if we
> do the `varbind X1` before or after `varbind X2` as long as they get
> the right value and as long as we don't compute anything which depends
> on those vars in-between.

Yes, that is all true.  But the byte compiler generates code which does
the _evaluation_ of the values in the wrong order, according to the
description of let on page "Local Variables" of the elisp manual.

This is a bug in either the byte compiler or the documentation of let.
I would tend towards the first of these alternatives.

Whatever, that let in jit-lock--debug-fontify would probably be safer if
it were a let*.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).