4

Here is some asynchronous code in a cl-loop:

;;; foo.el ---                              -*- lexical-binding: t; -*-
(let ((my-list '(a b c)))
  (cl-loop for index below (length my-list)
           for value = (seq-elt my-list index)
           do (run-at-time 0 nil
                           (lambda () (message "index=%s,value=%s" index value)))))

This code prints "index=3,value=c" 3 times in the *Messages* buffer. This is not what I want. I would like to see "index=0,value=a", "index=1,value=b", "index=2,value=c" (I don't care about the order of messages).

A workaround is to let-bind the index and value variables:

;;; foo.el ---                              -*- lexical-binding: t; -*-
(let ((my-list '(a b c)))
  (cl-loop for index below (length my-list)
           for value = (seq-elt my-list index)
           do (let ((index index) (value value))
                (run-at-time 0 nil
                             (lambda () (message "index=%s,value=%s" index value))))))

This is good enough for me but I thought there might be a more elegant solution.

Another workaround is to pass #'message as third argument to run-at-time instead of the lambda and to pass the arguments to message as &rest arguments to run-at-time. This is not good enough for me though because my real code is a bit more complex and involves calling a callback variable which may or may not do a run-at-time.

Damien Cassou
  • 877
  • 4
  • 14
  • IMO, your workaround would be the idiomatic way to get lexically scoped variables (in fact all the variables are already lexical, you just need a narrower scope). – npostavs Aug 20 '19 at 11:36
  • By the way, `for value = (seq-elt my-list index)` is [O(n^2)](https://accidentallyquadratic.tumblr.com/), you should probably use `for value in my-list` instead. – npostavs Aug 20 '19 at 11:38
  • Regarding the use of `seq-elt`, I agree with you. I've just asked a new question: https://emacs.stackexchange.com/questions/52251/iterate-with-both-value-and-index – Damien Cassou Aug 21 '19 at 09:11
  • Given your real code, I think a more representative example would use `collect (lambda () (message "value%s" value))` and then funcall the returned lambdas outside the loop. But that would invalidate @Tobias' answer. – npostavs Aug 21 '19 at 11:44

3 Answers3

3

Your problem is that the timer runs the function when the loop has finished. The lexically bound variables index and value have for all calls the values after the last iteration.

One solution is to evaluate the variables at iteration time and pass just the values as arguments to the function.

(let ((my-list '(a b c)))
  (cl-loop for index below (length my-list)
           for value = (seq-elt my-list index)
           do (run-at-time 0 nil
                           (lambda (index value) (message "index=%s,value=%s" index value))
               index value)))

With that method you could even free the function from its lexical environment:

(defun my-out (index value)
  (message "index=%s,value=%s" index value))

(let ((my-list '(a b c)))
  (cl-loop for index below (length my-list)
           for value = (seq-elt my-list index)
           do (run-at-time 0 nil
               #'my-out
               index value)))
Tobias
  • 32,569
  • 1
  • 34
  • 75
  • I like this answer best; although as the desired code "may or may not do a run-at-time", I'd suggest a slightly more generalised variant of "define external function accepting two arguments, and call it directly in the loop with: `do (my-func index value)` – phils Aug 20 '19 at 11:07
  • I have edited my question with a link to the real code so you see what I meant with "involves calling a callback variable which may or may not do a run-at-time". – Damien Cassou Aug 21 '19 at 09:16
  • @DamienCassou In your real code, the `run-at-time` part doesn't use `index` or `value`, so it's not really relevant (and you can't use this answer). Also, you only need to re-bind `index`, since `item` is used immediately. – npostavs Aug 21 '19 at 11:37
  • @npostavs I think the lambda itself is the problem. One does not know when the lambda is called (indirectly) through `mapfn`. It may be that `mapfn` also uses `run-at-time` for the call of the lambda. – Tobias Aug 21 '19 at 11:43
  • @Tobias indeed, `mapfn` may call its callback long after it has returned. That's the whole purpose of the `async-map` function I'm writing. – Damien Cassou Aug 22 '19 at 13:38
2

Another option is to use backquoting to substitute the values for each loop iteration directly into the lambda form:

;;; foo.el ---                              -*- lexical-binding: t; -*-
(let ((my-list '(a b c)))
  (cl-loop for index below (length my-list)
           for value = (seq-elt my-list index)
           do (run-at-time 0 nil
                           `(lambda () (message "index=%s,value=%s" ',index ',value)))))

But really, the approach you came up with yourself (i.e. to create lexical variables within each iteration) is probably the way to go.

This is, after all, a case of the classic "how do I create/fake a closure in elisp" question, and the idiomatic answer when lexical-binding is enabled is to use a let binding.

You don't need to repeat the same names for the inner variables, though -- and perhaps you'd find it less inelegant if you weren't doing that?

phils
  • 48,657
  • 3
  • 76
  • 115
  • If `run-at-time` would not allow for any arguments of the callback backquoting would be the best. +1 for the general approach. – Tobias Aug 21 '19 at 03:30
  • I have edited my question with a link to the real code so you see what I meant with "involves calling a callback variable which may or may not do a run-at-time". – Damien Cassou Aug 21 '19 at 09:16
  • I actually prefer having the same names in the `let` because it makes the workaround more explicit IMO. – Damien Cassou Aug 21 '19 at 09:18
1

One way is to create a macro:

(defmacro lexical-save (vars &rest body)
  `(lexical-let ,(mapcar (lambda (var) (list var var)) vars)
     ,@body))

You can use it is follows:

(let ((my-list '(a b c)))
  (cl-loop for index below (length my-list)
           for value = (seq-elt my-list index)
           do
           (run-at-time
            0 nil
            (lexical-save (index value)
                          (lambda () (insert (format "index=%s,value=%s\n" index value)))))))

I altered your example to let it insert text instead of using message, in order to better see what it's doing. Run it in the *scratch* buffer.

Harald Hanche-Olsen
  • 2,401
  • 12
  • 15