[babashka#379] Fix false dynamic binding

bbss · Aug 18, 2020 · c6e5369 · c6e5369
1 parent 4a819ce
commit c6e5369
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 22 deletions.
diff --git a/src/sci/impl/vars.cljc b/src/sci/impl/vars.cljc
@@ -64,6 +64,13 @@
   #?(:clj (.get dvals)
      :cljs @dvals))
 
+(deftype TBox #?(:clj [thread ^:volatile-mutable val]
+                 :cljs [thread ^:mutable val])
+  t/IBox
+  (setVal [this v]
+    (set! val v))
+  (getVal [this] val))
+
 (defn clone-thread-binding-frame ^Frame []
   (let [^Frame f #?(:clj (.get dvals)
                     :cljs @dvals)]
@@ -73,13 +80,6 @@
   #?(:clj (.set dvals frame)
      :cljs (reset! dvals frame)))
 
-(deftype TBox #?(:clj [thread ^:volatile-mutable val]
-                 :cljs [thread ^:mutable val])
-  t/IBox
-  (setVal [this v]
-    (set! val  v))
-  (getVal [this] val))
-
 (declare var?)
 
 (defn dynamic-var? [v]
@@ -259,25 +259,24 @@
     (not (instance? SciUnbound root)))
   t/IBox
   (setVal [this v]
-    (let [b (get-thread-binding this)]
-      (if (some? b)
-        #?(:clj
-           (let [t (.-thread b)]
-             (if (not (identical? t (Thread/currentThread)))
-               (throw (new IllegalStateException
-                           (str "Can't change/establish root binding of " this " with set")))
-               (t/setVal b v)))
-           :cljs (t/setVal b v))
-        (throw (new #?(:clj IllegalStateException :cljs js/Error)
-                    (str "Can't change/establish root binding of " this " with set"))))))
+    (if-let [b (get-thread-binding this)]
+      #?(:clj
+         (let [t (.-thread b)]
+           (if (not (identical? t (Thread/currentThread)))
+             (throw (new IllegalStateException
+                         (format "Can't set!: %s from non-binding thread" sym)))
+             (t/setVal b v)))
+         :cljs (t/setVal b v))
+      (throw (new #?(:clj IllegalStateException :cljs js/Error)
+                  (str "Can't change/establish root binding of " this " with set")))))
   (getVal [this] root)
   #?(:clj clojure.lang.IDeref :cljs IDeref)
   (#?(:clj deref
       :cljs -deref) [this]
     (if thread-bound
-      (or (when-let [tbox (get-thread-binding this)]
-            (t/getVal tbox))
-          root)
+      (if-let [tbox (get-thread-binding this)]
+        (t/getVal tbox)
+        root)
       root))
   Object
   (toString [_]

diff --git a/test/sci/vars_test.cljc b/test/sci/vars_test.cljc
@@ -35,7 +35,10 @@
                                   #_#_:sci.impl/built-in true})]
       (is (= [11 10] (sci/eval-string
                       "[(binding [*x* (fn [] 11)] (*x*)) (*x*)]"
-                      {:bindings {'*x* x}}))))))
+                      {:bindings {'*x* x}})))))
+  (testing "dynamic binding of false works"
+    (is (false? (sci/eval-string
+                 "(def ^:dynamic x nil) (binding [x false] x)")))))
 
 (deftest redefine-var-test
   (is (= 11 (eval* "