Force bindings inside `let-flow` to be unique

[tanzoniteblack]

Without this, the following code can randomly pick "cat" or "dog" (though "dog" is far more common, making this bug hard to detect or reproduce).

@(let-flow [a "cat"
            a "dog"]
   a)

Note: There's a likelyhood that users updating to this code will need to update their code base due to it failing to compile. I (Ryan Smith) find this acceptable as the code is already broken, even if the user hasn't realized it yet.

[tanzoniteblack]

This bug caused one of the most headache inducing bugs that I've ever encountered. Running API servers at Yummly returned different results using the exact same code and data. And to make it worse, when running the code in a repl & adding logging statements to try and debug, each time the code was recompiled it had the potential to randomly switch the results from expected to non.

[ztellman]

I'm sorry for the headache, but I think the correct thing to do here is to let variables be shadowed the way they would be in a normal let form, such that a in the body is whatever the last-bound a is. I'm assuming this is what you originally expected?

[tanzoniteblack]

I'm sorry for the headache, but I think the correct thing to do here is to let variables be shadowed the way they would be in a normal let form, such that a in the body is whatever the last-bound a is. I'm assuming this is what you originally expected?

This would be the desired behavior. The reason I went with this approach when we found the bug last year was a simple "I couldn't figure out a sane way to do this" inside the macro. I'm confident it's possible, it just wasn't something that I came up with a solution for in a reasonable period of time.

[youngilchoo]

We can make the bindings unique by turning them into a map, then back again. So, add

bindings2   (flatten
    (map
      (fn [[k v]] [(symbol k) v])
      (into {} (map
                 (fn [[s v]] [(keyword s) v])
                 (partition 2 bindings)))))

and use this in the definition for vars and vals'.

[tanzoniteblack]

@youngilchoo , there's an easier way to force the property of removing duplicate bindings to make only distinct names without throwing an exception (bindings (distinct bindings)). The problem with this method is that it doesn't cause shadowing, but just replaces.

@(let-flow [a (future "cat")
            b (future a)
            a "dog"]
   [a b])
=> ["cat" "cat"]

compare with let, which is the desired behavior:

(let [a "cat"
      b a
      a "dog"]
  [a b])
=> ["dog" "cat"]

[youngilchoo]

Good point. Not used to using let bindings like a sequence of assignments!

[KingMob]

Probably need to do something like rename the bindings based on static single assignment. It should be simpler, since there's no need for φ functions, since there's no branching in a let.