From 5145595d574bd82a09f4944be0e94d647fb2e4dc Mon Sep 17 00:00:00 2001 From: Gary Talent Date: Sat, 25 Jan 2025 22:16:42 -0600 Subject: [PATCH] [ox/std] Fix HashMap collision handling --- deps/ox/src/ox/std/hashmap.hpp | 132 ++++++++++++++++-------------- deps/ox/src/ox/std/test/tests.cpp | 10 +++ 2 files changed, 80 insertions(+), 62 deletions(-) diff --git a/deps/ox/src/ox/std/hashmap.hpp b/deps/ox/src/ox/std/hashmap.hpp index f2fbb599..14a53927 100644 --- a/deps/ox/src/ox/std/hashmap.hpp +++ b/deps/ox/src/ox/std/hashmap.hpp @@ -11,6 +11,7 @@ #include "algorithm.hpp" #include "hash.hpp" #include "ignore.hpp" +#include "optional.hpp" #include "stringview.hpp" #include "strops.hpp" #include "vector.hpp" @@ -26,11 +27,12 @@ class HashMap { private: struct Pair { + UPtr next; K key = {}; T value{}; }; Vector m_keys; - Vector m_pairs; + Vector> m_pairs; public: explicit constexpr HashMap(std::size_t size = 127); @@ -73,10 +75,10 @@ class HashMap { constexpr void expand(); template - constexpr Pair *const&access(Vector const&pairs, KK const&key) const; + constexpr UPtr const &access(Vector> const &pairs, KK const &key) const; template - constexpr Pair *&access(Vector &pairs, KK const&key); + constexpr UPtr &access(Vector> &pairs, KK const &key); }; @@ -85,14 +87,13 @@ constexpr HashMap::HashMap(std::size_t size): m_pairs(size) { } template -constexpr HashMap::HashMap(HashMap const&other) { - m_pairs = other.m_pairs; +constexpr HashMap::HashMap(HashMap const &other) { + operator=(other); } template -constexpr HashMap::HashMap(HashMap &&other) noexcept { - m_keys = std::move(other.m_keys); - m_pairs = std::move(other.m_pairs); +constexpr HashMap::HashMap(HashMap &&other) noexcept { + operator=(std::move(other)); } template @@ -101,7 +102,7 @@ constexpr HashMap::~HashMap() { } template -constexpr bool HashMap::operator==(HashMap const&other) const { +constexpr bool HashMap::operator==(HashMap const &other) const { if (m_keys != other.m_keys) { return false; } @@ -115,19 +116,25 @@ constexpr bool HashMap::operator==(HashMap const&other) const { } template -constexpr HashMap &HashMap::operator=(HashMap const&other) { +constexpr HashMap &HashMap::operator=(HashMap const &other) { if (this != &other) { clear(); m_keys = other.m_keys; - m_pairs = other.m_pairs; + m_pairs.resize(other.m_pairs.size()); + for (auto const&k : m_keys) { + auto const &src = access(other.m_pairs, k); + auto &dst = access(m_pairs, k); + dst = ox::make_unique(); + dst->key = src->key; + dst->value = src->value; + } } return *this; } template -constexpr HashMap &HashMap::operator=(HashMap &&other) noexcept { +constexpr HashMap &HashMap::operator=(HashMap &&other) noexcept { if (this != &other) { - clear(); m_keys = std::move(other.m_keys); m_pairs = std::move(other.m_pairs); } @@ -135,60 +142,52 @@ constexpr HashMap &HashMap::operator=(HashMap &&other) noexcep } template -constexpr T &HashMap::operator[](MaybeView_t const&k) { - auto p = &access(m_pairs, k); +constexpr T &HashMap::operator[](MaybeView_t const &key) { + auto p = &access(m_pairs, key); if (*p == nullptr) { if (static_cast(m_pairs.size()) * 0.7 < static_cast(m_keys.size())) { expand(); - p = &access(m_pairs, k); + p = &access(m_pairs, key); } - *p = new Pair; - (*p)->key = k; - m_keys.emplace_back(k); + *p = ox::make_unique(); + (*p)->key = key; + m_keys.emplace_back(key); } return (*p)->value; } template -constexpr Result HashMap::at(MaybeView_t const&k) noexcept { - auto p = access(m_pairs, k); +constexpr Result HashMap::at(MaybeView_t const &key) noexcept { + auto &p = access(m_pairs, key); if (!p) { - return {nullptr, ox::Error(1, "value not found for given key")}; + return ox::Error{1, "value not found for given key"}; } return &p->value; } template -constexpr Result HashMap::at(MaybeView_t const&k) const noexcept { - auto p = access(m_pairs, k); +constexpr Result HashMap::at(MaybeView_t const &key) const noexcept { + auto &p = access(m_pairs, key); if (!p) { - return {nullptr, ox::Error(1, "value not found for given key")}; + return ox::Error{1, "value not found for given key"}; } return &p->value; } template -constexpr void HashMap::erase(MaybeView_t const&k) { - if (!contains(k)) { +constexpr void HashMap::erase(MaybeView_t const &key) { + if (!contains(key)) { return; } - auto h = ox::hash>{}(k) % m_pairs.size(); - while (true) { - const auto &p = m_pairs[h]; - if (p == nullptr || p->key == k) { - std::ignore = m_pairs.erase(h); - break; - } else { - h = ox::hash>{}(k) % m_pairs.size(); - } - } - std::ignore = m_keys.erase(ox::find(m_keys.begin(), m_keys.end(), k)); + auto &c = access(m_pairs, key); + c = std::move(c->next); + std::ignore = m_keys.erase(ox::find(m_keys.begin(), m_keys.end(), key)); } template -constexpr bool HashMap::contains(MaybeView_t const&k) const noexcept { - return access(m_pairs, k) != nullptr; +constexpr bool HashMap::contains(MaybeView_t const &key) const noexcept { + return access(m_pairs, key).get() != nullptr; } template @@ -204,27 +203,26 @@ constexpr Vector const&HashMap::keys() const noexcept { template constexpr Vector HashMap::values() const noexcept { Vector out; - out.reserve(m_pairs.size()); - for (auto const&p : m_pairs) { - out.emplace_back(p->value); + out.reserve(m_keys.size()); + for (auto const &p : m_pairs) { + if (out) { + out.emplace_back(p->value); + } } return out; } template constexpr void HashMap::clear() { - for (std::size_t i = 0; i < m_pairs.size(); i++) { - delete m_pairs[i]; - } m_pairs.clear(); m_pairs.resize(127); } template constexpr void HashMap::expand() { - Vector r(m_pairs.size() * 2); + Vector> r{m_pairs.size() * 2}; for (std::size_t i = 0; i < m_keys.size(); ++i) { - auto const&k = m_keys[i]; + auto const &k = m_keys[i]; access(r, k) = std::move(access(m_pairs, k)); } m_pairs = std::move(r); @@ -232,29 +230,39 @@ constexpr void HashMap::expand() { template template -constexpr typename HashMap::Pair *const&HashMap::access(Vector const&pairs, KK const&k) const { - auto h = static_cast(ox::hash{}(k) % pairs.size()); +constexpr UPtr::Pair> const &HashMap::access( + Vector> const& pairs, + KK const &key) const { + auto const h = static_cast(ox::hash{}(key) % pairs.size()); + auto const &p = *pairs.at(h).unwrap(); + if (p == nullptr || p->key == key) { + return p; + } + auto c = &p->next; while (true) { - auto const&p = *pairs.at(h).unwrap(); - if (p == nullptr || p->key == k) { - return p; - } else { - h = (h + 1) % pairs.size(); + if (*c == nullptr || (*c)->key == key) { + return *c; } + c = &(*c)->next; } } template template -constexpr typename HashMap::Pair *&HashMap::access(Vector &pairs, KK const&k) { - auto h = static_cast(ox::hash{}(k) % pairs.size()); +constexpr UPtr::Pair> &HashMap::access( + Vector> &pairs, + KK const &key) { + auto const h = static_cast(ox::hash{}(key) % pairs.size()); + auto &p = *pairs.at(h).unwrap(); + if (p == nullptr || p->key == key) { + return p; + } + auto c = &p->next; while (true) { - auto &p = *pairs.at(h).unwrap(); - if (p == nullptr || p->key == k) { - return p; - } else { - h = (h + 1) % pairs.size(); + if (*c == nullptr || (*c)->key == key) { + return *c; } + c = &(*c)->next; } } diff --git a/deps/ox/src/ox/std/test/tests.cpp b/deps/ox/src/ox/std/test/tests.cpp index 68f5252f..d999d39d 100644 --- a/deps/ox/src/ox/std/test/tests.cpp +++ b/deps/ox/src/ox/std/test/tests.cpp @@ -328,6 +328,16 @@ OX_CLANG_NOWARN_END si["aoeu"] = 100; oxAssert(si["asdf"] == 42, "asdf != 42"); oxAssert(si["aoeu"] == 100, "aoeu != 100"); + si.erase("asdf"); + oxAssert(!si.contains("asdf"), "wrongly contains asdf"); + oxAssert(si.contains("aoeu"), "does not contains aoeu"); + oxAssert(!si.at("asdf").ok(), "asdf != 0"); + oxExpect(si["asdf"], 0); + oxAssert(si["aoeu"] == 100, "aoeu != 100"); + auto si2 = si; + oxDebugf("{}", si2["asdf"]); + oxExpect(si2["asdf"], 0); + oxAssert(si2["aoeu"] == 100, "aoeu != 100"); ox::HashMap ii; ii[4] = 42; ii[5] = 100;