top | item 37577790

(no title)

travisd | 2 years ago

It's worth noting that it's much less of a problem in Python due to the lack of ergonomic closures/lambdas. You have to construct rather esoteric looking code for it to be a problem.

    add_n = []
    for n in range(10):
        add_n.append(lambda x: x + n)
    add_n[9](10)  # 19
    add_n[0](10)  # 19
This isn't to say it's *not* a footgun (and it has bit me in Python before), but it's much worse in Go due to the idiomatic use of goroutines in a loop:

    for i := 0; i < 10; i++ {
        go func() { fmt.Printf("num: %d\n", i) }()
    }

discuss

order

eru|2 years ago

In Python you are much more likely to hit that problem not with closures constructed with an explicit 'lambda', but with generator-comprehension expressions.

    (((i, j) for i in "abc") for j in range(3))
The values of the above depends on in which order you evaluate the whole thing.

(Do take what I wrote with a grain of salt. Either the above is already a problem, or perhaps you also need to mix in list-comprehension expressions, too, to surface the bug.)

nerdponx|2 years ago

Yeah, this one is weird:

  gs1 = (((i, j) for i in "abc") for j in range(3))
  gs2 = [((i, j) for i in "abc") for j in range(3)]

  print(list(map(list, gs1)))
  print(list(map(list, gs2)))
Results:

  [[('a', 0), ('b', 0), ('c', 0)], [('a', 1), ('b', 1), ('c', 1)], [('a', 2), ('b', 2), ('c', 2)]]
  [[('a', 2), ('b', 2), ('c', 2)], [('a', 2), ('b', 2), ('c', 2)], [('a', 2), ('b', 2), ('c', 2)]]
That's a nice "wat" right there. I believe the explanation is that in gs2, the range() is iterated through immediately, so j is always set to 2 before you have a chance to access any of the inner generators. Whereas in gs1 the range() is still being iterated over as you access each inner generator, so when you access the first generator j=1, then j=2, etc.

Equivalents:

  def make_gs1():
      for j in range(2):
          yield ((i, j) for i in "abc")

  def make_gs2():
      gs = []
      for j in range(2):
          gs.append(((i, j) for i in "abc"))
      return gs
Late binding applies in both cases of course, but in the first case it doesn't matter, whereas in the latter case it matters.

I think early binding would produce the same result in both cases.

Spivak|2 years ago

Ignoring the strange nature of this code in the first place the more pythonic way to do it would be

    from functools import partial
    from operators import add

    add_n = [partial(add, n)) for n in range(10)]

    assert add_n[5](4) == 9
Look ma, no closures.

eru|2 years ago

'partial' creates a closure for you.

hinkley|2 years ago

Everyone else solved this problem by using list comprehensions instead. Rob has surely heard of those.

lmm|2 years ago

Of the two comprehension syntaxes in Haskell, Python picked the wrong one. Do notation (or, equivalently, Scala-style for/yield) feels much more consistent and easy to use - in particular the clauses are in the same order as a regular for loop, rather than the middle-endian order used by list comprehensions.

panzi|2 years ago

How does list comprehension change anything here? This has the same problem:

    add_n = [lambda x: x + n for n in range(10)]
    add_n[9](10)  # 19
    add_n[0](10)  # 19

simiones|2 years ago

I don't think anyone is puzzled by the Go snippet being wrong.

The bigger problem in Go is the for with range loop:

  pointersToV := make([]*val, len(values))
  for i, v := range values {
    go func() { fmt.Printf("num: %v\n", v) } () //race condition
    pointersToV[i] = &v //will contain len(values) copies of a pointer to the last item in values
  }
This is the one they are changing.

Edit: it looks like they're actually changing both of these, which is more unexpected to me. I think the C# behavior makes more sense, where only the foreach loop has a new binding of the variable in each iteration, but the normal for loop has a single lifetime.

o11c|2 years ago

It's actually worse in Python since there's no support for variable lifetimes within a function, so the `v2` workaround is still broken. (the default-argument workaround "works" but is scary)

This makes it clear: the underlying problem is NOT about for loops - it's closures that are broken.

nerdponx|2 years ago

It's not broken, it's a different design. Maybe worse in a lot of cases, but it's not broken. It's working as intended.

jshen|2 years ago

Doesn’t go vet complain about your code? I’m not at my computer right now so can’t check.

_ikke_|2 years ago

> Tools have been written to identify these mistakes, but it is hard to analyze whether references to a variable outlive its iteration or not. These tools must choose between false negatives and false positives. The loopclosure analyzer used by go vet and gopls opts for false negatives, only reporting when it is sure there is a problem but missing others.

So it will warn in certain situations, but not all of them

simiones|2 years ago

Why would it? It's perfectly correct code, it's just not doing what you'd expect.

It might complain about the race condition, to be fair, but the same issue can be reproduced without goroutines and it would be completely correct code per the semantics.

Frotag|2 years ago

I've run into this once. IIRC the workaround was to add a n=n arg to the lambda