(CLJS) the binding macro/let binding is wrongly inlined

Question

{quote}
(def ^:dynamic foo 42)

(go

(let (link: old foo)
  (set! foo 45)
  (println old foo)
  (set! foo old)))

{quote}

leaves the binding with the value 45 (same code as with binding+with-redefs). The problem is that the let binding is somehow inlining the reference to foo. For instance the println statement compiles to:

{quote}
...
var inst_43089 = cljs.core.println.call(null,full.async.foo,full.async.foo);
...
{quote}

I am currently having a look at ioc_macros.clj, but I couldn't find the problematic part yet. Any hints are helpful.

Answer 1

_Comment made by: hiredman_

for the curious the cleaned up macro expansion of the above (in clojurescript) is:


(let* [c__9201__auto__ (chan 1)]
      (run
        (fn []
          (let [f__9202__auto__ (let [switch__9186__auto__ (fn [state_9258]
                                                             (let [state_val_9259 (aget state_9258 1)]
                                                               (cond
                                                                (== state_val_9259 1) (let [inst_9254 (set! foo 45)
                                                                                            inst_9255 (println foo foo)
                                                                                            inst_9256 (set! foo foo)
                                                                                            state_9258 (aset-all! state_9258 7 inst_9255 8 inst_9254)]
                                                                                        (return-chan state_9258 inst_9256)))))]
                                  (fn state-machine__9187__auto__
                                    ([] (aset-all! (make-array 9) 0 state-machine__9187__auto__ 1 1))
                                    ([state_9258] (let [ret-value__9188__auto__ (try
                                                                                  (loop []
                                                                                    (let [result__9189__auto__ (switch__9186__auto__ state_9258)]
                                                                                      (if (keyword-identical? result__9189__auto__ :recur)
                                                                                        (recur)
                                                                                        result__9189__auto__)))
                                                                                  (catch js/Object ex__9190__auto__
                                                                                    (aset-all! state_9258 5 ex__9190__auto__)
                                                                                    (process-exception state_9258)
                                                                                    :recur))]
                                                    (if (keyword-identical? ret-value__9188__auto__ :recur)
                                                      (recur state_9258)
                                                      ret-value__9188__auto__)))))
                state__9203__auto__ (-> (f__9202__auto__)
                                        (aset-all! USER-START-IDX c__9201__auto__))]
            (run-state-machine-wrapped state__9203__auto__))))
      c__9201__auto__)




the issue is definitely present in the macro expansion

Answer 2

Comment made by: hiredman

it looks like the issue is, let binds essentially disappear at runtime
because the ioc macros bind every expression to a name, so let binds
are just mapped to those names at compile time. in that mapping global
names essentially map to themselves, so let bound names that get their
value from a global just disappear and become references to the
global.

If you look at the :symbol case for -item-to-ssa in
ioc_macros.clj, there is a commented out (add-instruction (->Const x)), if you uncomment out that and comment out the fn above it, I
think what you get out has the behavior you are looking for, at the
cost of creating a local for every global read.

You could do some kind of picking of the behavior based on if the
globals are declared to be dynamic, but that would still leave issues
for with-redefs. In JVM clojure with-redefs works on global names
regardless of if they are dynamic or not. You could generate some kind
of dirty set if you see a set! of a global, and use that to toggle the
behavior, but I think (not sure) that could run in to issues because
the analysis is local in scope.

Answer 3

_Comment made by: hiredman_

if you make the change I mentioned in my comment above, the relevant section of the cleaned up macro expansion looks like:


(let [inst_9257 foo
      inst_9258 foo
      inst_9259 (set! inst_9258 45)
      inst_9260 println
      inst_9261 foo
      inst_9262 (inst_9260 inst_9257 inst_9261)
      inst_9263 foo
      inst_9264 (set! inst_9263 inst_9257)
      state_9266 (aset-all! state_9266 7 inst_9259 8 inst_9262)]
  (return-chan state_9266 inst_9264))



that actually has a bug too, it turns out, because the set! is changing the value of a local instead of the global `foo`

Answer 4

Comment made by: hiredman

someone should check to see if the clojure ioc macros do this same
thing, because you could have a similar issue, something like:

`
(def ^:dynamic foo 42)

(go (binding [foo 5] (let [x foo] (set! foo 20) (println x)))
`

I would expect that to print 5, if there is some kind of aliasing bug, it might print 20

Answer 5

Comment made by: hiredman

the 001 patch causes local bindings that are initialized from a global to actually create a local and initialize it from the global, instead of reading from the global.

Answer 6

Comment made by: hiredman

patch 0002 is patch 0001, but using the same code path in let binding inits and loops binding inits

Answer 7

Comment made by: hiredman

0003 adds a test for the expected local binding behavior when aliasing a global

Answer 8

Reference: https://clojure.atlassian.net/browse/ASYNC-165 (reported by alex+import)