From 630eb2b9d3e3cf366e6f2a14f60480d55ff25eb6 Mon Sep 17 00:00:00 2001 From: Dmitri Sotnikov Date: Wed, 17 Jun 2026 01:43:23 +0000 Subject: [PATCH] Keep a nil element in sets and transient sets (jolt-bn2p) (#156) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/jolt/core_coll.janet | 4 ++-- src/jolt/core_extra.janet | 8 ++++---- src/jolt/core_types.janet | 8 ++++++++ src/jolt/phm.janet | 2 ++ src/jolt/phs.janet | 7 ++++++- test/spec/sets-spec.janet | 21 +++++++++++++++++++++ 6 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/jolt/core_coll.janet b/src/jolt/core_coll.janet index 2c16041..20804e4 100644 --- a/src/jolt/core_coll.janet +++ b/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))) diff --git a/src/jolt/core_extra.janet b/src/jolt/core_extra.janet index c73e1da..d8220d5 100644 --- a/src/jolt/core_extra.janet +++ b/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) diff --git a/src/jolt/core_types.janet b/src/jolt/core_types.janet index 48868ef..c17a20a 100644 --- a/src/jolt/core_types.janet +++ b/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. diff --git a/src/jolt/phm.janet b/src/jolt/phm.janet index 7a6f297..bfbc609 100644 --- a/src/jolt/phm.janet +++ b/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 diff --git a/src/jolt/phs.janet b/src/jolt/phs.janet index fe026e6..c28c62a 100644 --- a/src/jolt/phs.janet +++ b/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) diff --git a/test/spec/sets-spec.janet b/test/spec/sets-spec.janet index 5ba1eee..e041010 100644 --- a/test/spec/sets-spec.janet +++ b/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}))"]