Open MartyO256 opened 1 year ago
Thanks for the nice, detailed and reproducible report! Unfortunately there is very few (people.time) allocated to working on hamt at the time, so I'm not sure when we'll be able to fix it. I've been looking briefly at the code but wasn't able to find the obvious mistake.
If someone wants to reproduce and propose a patch (that I'll happily accept): all four items on the add_all_by_union'
are needed to trigger the issue. Which probably points to a bug on merging BitmapIndexedNodes
.
Expected Behavior
If HAMT
m
is built by taking the union of two HAMTsm1
andm2
, then we should be able to find a binding inm
for each key inm1
andm2
.For instance, the following two functions should be equivalent:
Current Behavior
Some entries cannot be found using
Hamt.String.find_opt
orHamt.String.mem
when we useadd_all_by_union
. The bindings are correctly present when usingHamt.String.bindings
.This suggests that HAMTs obtained using
union
are not necessarily built in the wayfind_opt
expects.Steps to Reproduce
In the following example, we're building the HAMT with bindings
[("tp", ()); ("suc", ()); ("something", ()); ("expCtx", ())]
as the union of the HAMTs with bindings[("tp", ()); ("suc", ())]
and[("something", ()); ("expCtx", ())]
respectively. WhileHamt.String.bindings hamt = [("something", ()); ("suc", ()); ("tp", ()); ("expCtx", ())]
does contain all the bindings, none of the keys can be found withHamt.String.mem
.This prints out:
Environment
OCaml 4.14.1 Hamt revision 2d5e5364a12e7b9bb61ad5dfaf6062353d942e26