Keep a nil element in sets and transient sets (jolt-bn2p) (#156)

Two paths dropped a nil set member. phs-seq read members via
phm-to-struct, whose Janet struct can't hold a nil key, so the nil was
lost on seq/print and on into-an-existing-set even though the backing phm
counted it (count/contains? then disagreed). Re-attach the nil from the
phm's has-nil slot, keeping struct-key order for the rest so set printing
stays stable.

The transient set keyed its native table by canon-key and stored the
member as the value; canon-key returns nil for nil and a nil value also
drops the entry, so conj!/disj!/contains?/persistent! lost a nil member
outright. Key by tbl-key (the same nil sentinel the transient map uses)
and box a nil value through tbl-nil-key, unboxing on persistent!.

A nil-containing set used as a map key still collides with the nil-free
one (canon-key drops nil during key canonicalization) — separate latent
bug, filed as jolt-zcm9.

Co-authored-by: Yogthos <yogthos@gmail.com>

src/jolt/core_coll.janet

@@ -183,7 +183,7 @@
       (case (m :kind)
         :vector (if (and (number? k) (>= k 0) (< k (length (m :arr)))) (in (m :arr) k) default)
         :map (let [p (get (m :tbl) (tbl-key k))] (if p (in p 1) default))
-        :set (if (nil? (get (m :tbl) (canon-key k))) default k))
+        :set (if (nil? (get (m :tbl) (tbl-key k))) default k))
     (if (set? m) (phs-get m k default)
       (if (phm? m) (phm-get m k default)
         (if (pvec? m)
@@ -251,7 +251,7 @@
   (if (core-transient? coll)
     (case (coll :kind)
       :vector (and (number? key) (>= key 0) (< key (length (coll :arr))))
-      (not (nil? (get (coll :tbl) (if (= :map (coll :kind)) (tbl-key key) (canon-key key))))))
+      (not (nil? (get (coll :tbl) (tbl-key key)))))
   (if (set? coll) (phs-contains? coll key)
     (if (phm? coll) (phm-contains? coll key)
       (if (pvec? coll) (and (number? key) (>= key 0) (< key (pv-count coll)))

src/jolt/core_extra.janet

@@ -338,7 +338,7 @@
     (pvec? coll)
       @{:jolt/type :jolt/transient :kind :vector :arr (pv->array coll)}
     (set? coll)
-      (let [t @{}] (each e (phs-seq coll) (put t (canon-key e) e))
+      (let [t @{}] (each e (phs-seq coll) (put t (tbl-key e) (tbl-box e)))
         @{:jolt/type :jolt/transient :kind :set :tbl t})
     (or (phm? coll) (and (struct? coll) (nil? (get coll :jolt/type))))
       (let [t @{}]
@@ -360,7 +360,7 @@
   (tr-check-active! t)
   (case (t :kind)
     :vector (array/push (t :arr) x)
-    :set    (put (t :tbl) (canon-key x) x)
+    :set    (put (t :tbl) (tbl-key x) (tbl-box x))
     :map    (cond
               # a [k v] pair (map-entry / 2-vector)
               (and (or (pvec? x) (tuple? x) (array? x))
@@ -410,7 +410,7 @@
 
 (defn core-disj! [t & xs]
   (if (and (core-transient? t) (= :set (t :kind)))
-    (do (tr-check-active! t) (each x xs (put (t :tbl) (canon-key x) nil)) t)
+    (do (tr-check-active! t) (each x xs (put (t :tbl) (tbl-key x) nil)) t)
     (error "disj! requires a transient set")))
 
 (defn core-pop! [t]
@@ -431,7 +431,7 @@
           # persistent value ONCE (bottom-up HAMT) instead of folding a phm-assoc
           # per entry. This is the lever behind every transient-based builder
           # (frequencies/group-by/set/into) — jolt-5vsp collections.
-          :set (phs-from-seq (values (t :tbl)))
+          :set (phs-from-seq (map tbl-unbox (values (t :tbl))))
           :map (phm-from-pairs (values (t :tbl)))))
       # Invalidate: any further bang op (or a second persistent!) now throws.
       (put t :jolt/persistent true)

src/jolt/core_types.janet

@@ -104,6 +104,14 @@
 (def tbl-nil-key @{})
 (defn tbl-key [k] (if (nil? k) tbl-nil-key (canon-key k)))
 
+# A transient SET stores `(tbl-key x) -> x`, i.e. the member IS the table value.
+# A nil member can't be a Janet table value either (put with a nil value drops
+# the entry), so box nil as the same sentinel and unbox on read-back. The
+# sentinel is a fresh table canon-key never produces, so it can't collide with a
+# real member.
+(defn tbl-box [x] (if (nil? x) tbl-nil-key x))
+(defn tbl-unbox [v] (if (= v tbl-nil-key) nil v))
+
 # All [k v] entries of a map (struct or phm), nil-valued keys included. Use this
 # instead of (keys (phm-to-struct m)) — phm-to-struct drops keys whose value is
 # nil, which is exactly what Clojure maps must keep.

src/jolt/phm.janet

@@ -297,6 +297,8 @@
 
 (defn phm-count [m] (m :cnt))
 
+(defn phm-has-nil? [m] (truthy? (m :has-nil)))
+
 # --- bulk bottom-up build (jolt-5vsp collections) ---------------------------
 # Build the HAMT in one pass from a native array of entries, instead of n
 # incremental phm-assoc calls (each of which rebuilt the O(log32 n) path with

src/jolt/phs.janet

@@ -48,7 +48,12 @@
   (= 0 (s :cnt)))
 
 (defn phs-seq [s]
-  (tuple ;(keys (phm-to-struct (s :phm)))))
+  # phm-to-struct drops a nil member (a Janet struct can't hold a nil key), so
+  # re-attach it from the phm's has-nil slot. Keep the struct-key order for the
+  # non-nil members so set printing stays stable.
+  (def m (s :phm))
+  (def ks (keys (phm-to-struct m)))
+  (if (phm-has-nil? m) (tuple nil ;ks) (tuple ;ks)))
 
 (defn phs-get [s x &opt default]
   (default default nil)

test/spec/sets-spec.janet

@@ -33,6 +33,27 @@
   ["dedup equal maps"       "1"         "(count (set [{:a 1} (hash-map :a 1)]))"]
   ["vector elements"        "true"      "(contains? #{[1 2]} (vec [1 2]))"])
 
+(defspec "set / nil element (jolt-bn2p)"
+  # canon-key returns nil for nil and Janet tables drop a nil key, so a nil
+  # member used to be silently lost while count/contains? disagreed.
+  ["set keeps nil"            "2"     "(count (set [nil 1 nil]))"]
+  ["contains? nil true"       "true"  "(contains? (set [nil 1]) nil)"]
+  ["contains? nil false"      "false" "(contains? #{1} nil)"]
+  ["seq includes nil"         "true"  "(some nil? (seq (set [nil 1])))"]
+  ["disj nil"                 "#{1}"  "(disj (set [nil 1]) nil)"]
+  ["disj nil count"           "1"     "(count (disj (set [nil 1]) nil))"]
+  ["conj nil count"           "2"     "(count (conj #{1} nil))"]
+  ["conj nil contains?"       "true"  "(contains? (conj #{1} nil) nil)"]
+  ["into #{} keeps nil"       "2"     "(count (into #{} [nil 1]))"]
+  ["into #{} contains? nil"   "true"  "(contains? (into #{} [nil 1]) nil)"]
+  ["into keeps existing nil"  "true"  "(contains? (into #{nil} [1]) nil)"]
+  # transient set path: tr-conj!/persistent!/disj!/contains?
+  ["transient conj! nil"      "2"     "(count (persistent! (conj! (transient #{}) nil 1)))"]
+  ["transient contains? nil"  "true"  "(contains? (persistent! (conj! (transient #{}) nil 1)) nil)"]
+  ["transient disj! nil cnt"  "1"     "(count (persistent! (disj! (conj! (transient #{}) nil 1) nil)))"]
+  ["transient disj! removes"  "false" "(contains? (persistent! (disj! (conj! (transient #{}) nil 1) nil)) nil)"]
+  ["transient of set w/ nil"  "true"  "(contains? (persistent! (transient (set [nil 1]))) nil)"])
+
 (defspec "clojure.set"
   ["union"                  "#{1 2 3 4}" "(do (require (quote [clojure.set :as s])) (s/union #{1 2} #{3 4}))"]
   ["intersection"           "#{2}"       "(do (require (quote [clojure.set :as s])) (s/intersection #{1 2} #{2 3}))"]