Fork me on GitHub
#cljs-dev
<
2022-12-20
>
Sam Ritchie16:12:17

hey all, I am hitting a bug in cljs that I can recreate with shadow, but that I’m not sure how to recreate without shadow in the loop. The error is this:

Uncaught TypeError: Cannot create property 'closure_uid_21581550' on bigint '-12'
    at goog.getUid (cljs_env.js:392:127)
    at core.cljs:1436:6
    at cljs$core$IHash$_hash$dyn_28715 (core.cljs:724:1)
    at Object.cljs$core$_hash [as _hash] (core.cljs:724:1)
    at Object.cljs$core$hash [as hash] (core.cljs:1036:15)
    at Object.cljs$core$hash_ordered_coll [as hash_ordered_coll] (core.cljs:1360:54)
    at Object.cljs$core$IHash$_hash$arity$1 (core.cljs:3133:36)
    at Object.cljs$core$hash [as hash] (core.cljs:1008:21)
    at Object.cljs$core$hash_ordered_coll [as hash_ordered_coll] (core.cljs:1360:54)
    at Object.cljs$core$IHash$_hash$arity$1 (core.cljs:3133:36)
I’ve whittled down my reproduction to this:
(let [f (memoize identity)]
  (f (list 'a (list 'b (js/BigInt -12) 'c (list 'd)) 'e))
  (f 'a)
  (f (list 'b (js/BigInt -12) 'c (list 'd)) 'e)
  (f 'b)
  (f (js/BigInt -12))
  (f 'c)
  (f (list 'd))
  (f 'd)
  (f 'e))
I haven’t found a different form that does this, but obviously there is something important about the fact that if I pass • this nested form with a bigint inside • pass each sequence’s node in depth-first order to the memoized function then on the final entry, this error is thrown

Sam Ritchie16:12:24

cc @U05224H0W , here is a repro with shadow: https://github.com/sritchie/shadow-repro I can’t get this to trigger in a bare cljs browser repl with the same cljs version, so maybe it is something about the esm module settings:

{:deps true
 :dev-http {9000 "public"}
 :builds
 {:repro
  {:target :esm
   :runtime :browser
   :output-dir "public/js"
   :modules {:main
             {:entries [repro]}}
   :js-options
   {:output-feature-set :es8}}}}

Sam Ritchie17:12:51

AHHH wait I have a better repro and a bigger clue 🙂 this happens. If you call the memoized function with more than 8 distinct values, if 1 of the values is a BigInt, you get the failure.

(let [f (memoize identity)]
  (doseq [i (range 8)]
    (f i))
  (f (js/BigInt 1)))

p-himik17:12:52

While playing around with it, found a not-so-great behavior:

cljs.user=> (def i (js/BigInt -12))
#'cljs.user/i
cljs.user=> (hash i)
2
cljs.user=> (hash i)
3
cljs.user=> (hash i)
4

Sam Ritchie17:12:56

@U2FRKM4TW actually bigint is a red herring too

Sam Ritchie17:12:25

no, wait, it’s not, sorry. I thought it was because (-hash 10) fails with the same error

Sam Ritchie17:12:25

but yeah that is not good @U2FRKM4TW

Sam Ritchie17:12:27

(defn hash
  "Returns the hash code of its argument. Note this is the hash code
   consistent with =."
  [o]
  (cond
    (implements? IHash o)
    (bit-xor (-hash o) 0)

    (number? o)
    (if ^boolean (js/isFinite o)
      (js-mod (Math/floor o) 2147483647)
      (case o
        ##Inf
        2146435072
        ##-Inf
        -1048576
        2146959360))

Sam Ritchie17:12:29

numbers get a pass here

Sam Ritchie17:12:19

@U2FRKM4TW so in my setup, hash with a bigint totally fails. what version are you using?

p-himik17:12:27

Feels like failing on hashing BigInt would actually be better than always returning a different hash.

💯 1
p-himik17:12:42

BTW the behavior I described was with a stock REPL, without shadow-cljs.

Sam Ritchie17:12:04

(extend-type js/BigInt
  IHash
  (-hash
    [this] (hash (.toString this 16)))

  IEquiv
  (-equiv [this o]
    (let [other (.valueOf o)]
      (if (= "bigint" (goog/typeOf other))
        (coercive-= this other)
        (= this other)))))

Sam Ritchie17:12:54

I guess this is the key

Sam Ritchie17:12:52

not quite right…

p-himik17:12:55

I'd say this is a crutch. :) The key would be somehow solving it at the GCC/CLJS level.

niwinz07:12:34

The problem here is, that in JS, bigInt is a "primitive type" but in GCC/CLJS is treated as reference type, I guess the same happens with js/Symbol (I have had similar problems but with js/Symbol in the past, don't know if it solved)