Arc Forumnew | comments | leaders | submitlogin
2 points by pg 3982 days ago | link | parent

Thanks for catching this. I fixed both for and down.


3 points by pg 3979 days ago | link

Oops, spoke too soon. This new version would mean you can't modify the variable within the loop, which is something I meant to be possible, and in fact use in e.g. urldecode.

I believe the strange behavior palsecam discovered is actually correct. But if anyone wants to make the case that it shouldn't be, I'm open to being convinced.

-----

3 points by conanite 3979 days ago | link

I believe the strange behavior palsecam discovered is actually correct.

I believe the behaviour isn't even strange. Inside your for loop, (thread ...) creates a closure which references i, and when the closure is invoked, it looks up the current value of i, which has in the meantime changed.

  (def test-strange-behaviour ()
    (let fns (accum x
      (for i 0 10 (x (fn () (prn i)))))
    (each f fns (f))))

  (test-strange-behaviour) ; displays "11" 10 times
javascript has the same behaviour:

  <script type="text/javascript">
  var fns = [];

  function strange() {
    for (var i=0; i<3; i++) {
      fns[i] = function () { alert(i); }
    }

    for (var j = 0; j < 3; j++) {
      fns[j]();
    }
  }

  strange(); // alerts "3" 3 times
  </script>
The workaround is to outsource the closure-creation to another function:

  (def loop-work (i)
    (fn () (prn i)))

  (def no-strange-behaviour ()
    (let fns (accum x
      (for i 0 10 (x (loop-work i))))
    (each f fns (f))))

  (no-strange-behaviour) ; displays 0 up to 10
This works because now the closure references the i that belongs to the invocation of loop-work that created the closure; nothing modifies that i. The strangeness has nothing to do with threads; it's only about closures.

-----

1 point by palsecam 3979 days ago | link

Warning: quick (& dirty) reflexion and patch.

> you can't modify the variable within the loop, which is something I meant to be possible

Yes useful feature, so maybe:

  (mac for (v init max . body)
    (w/uniq (gv gi gm)
      `(with (,gv nil ,gi ,init ,gm (+ ,max 1))
         (loop (assign ,gv ,gi) (< ,gv ,gm) (assign ,gv (+ ,gv 1))
           ((fn (,v) ,@body (= ,gv ,v)) ,gv)))))
?

Very lightly tested, only in the online repl, but seems OK although a bit ugly.

   arc> (for i 0 10 (pr i " ") (++ i))
   0 2 4 6 8 10 nil
   arc> (do (for i 0 10 (thread:pr i " ")) (sleep 1))
   0 10 8 6 4 2 9 5 1 7 3 nil
   arc>	(urldecode "80%25%20-+20%25")
   "80% - 20%"
   
Anyway, it'd make the def of 'for more complex, less clean, and the perf a little bit worse.

> I believe the strange behavior palsecam discovered is actually correct. But if anyone wants to make the case that it shouldn't be, I'm open to being convinced.

I don't really care but I like to play the devil's advocate :-)

It's a bug for my brain. I'd sleep better at night if I knew I could use 'for in any situation, even w/ threads. 1: Simpler. The less stuff I have to keep in mind (e.g: "oh right, and remember 'for is not thread-safe"), the better. 2: More robust. I like to know I can "stand on the shoulders of giants" and that edge cases are handled correctly.

It's a bug because you call it "strange" and considered it as a bug (and so do I). Maybe we're wrong and we can't see the real problem(s) behing using threads in a loop construct, or maybe this behaviour is just a free overhead that shouldn't exist, and we're right.

-----