From 88194b9a5bc17ee5b97d360a5eb448e1c31fd883 Mon Sep 17 00:00:00 2001 From: Pagwin Date: Fri, 22 Nov 2024 17:50:16 -0500 Subject: [PATCH] fixed a bug due to using pointers instead of vector indices --- Map.hpp | 406 +++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 255 insertions(+), 151 deletions(-) diff --git a/Map.hpp b/Map.hpp index 155360d..a4e5191 100644 --- a/Map.hpp +++ b/Map.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -38,11 +39,11 @@ template struct BookKeeping { // Ptr self; Color color; // nullptr indicates empty - Self *parent; - Self *left; - Self *right; - Self *prev; - Self *next; + std::optional parent; + std::optional left; + std::optional right; + std::optional prev; + std::optional next; BookKeeping(Map &container) : container{container} {} BookKeeping(BookKeeping const &rhs) : container{rhs.container}, value{rhs.value}, // self{rhs.self}, @@ -65,7 +66,11 @@ template struct BookKeeping { return *this; } // reference to a pointer because the alternatives were worse - inline Self *&child(Direction dir) { + inline Self *child(Direction dir) { + auto ret = c_select(dir); + return ret.has_value() ? &container.nodes[left.value()] : nullptr; + } + inline std::optional c_select(Direction dir) { switch (dir) { case Direction::Left: return left; @@ -77,6 +82,64 @@ template struct BookKeeping { assert(false); } } + inline void c_trans(Direction dir, Self *v) { + switch (dir) { + case Direction::Left: + this->set_l(v); + break; + case Direction::Right: + this->set_r(v); + break; + default: + assert(false); + } + } + + inline Self *n() { + return next.has_value() ? &container.nodes[next.value()] : nullptr; + } + inline void set_n(Self *ptr) { + this->next = ptr == nullptr + ? std::nullopt + : std::optional{static_cast( + ptr - &this->container.nodes[0])}; + } + inline Self *p() { + return prev.has_value() ? &container.nodes[prev.value()] : nullptr; + } + inline void set_p(Self *ptr) { + this->prev = ptr == nullptr + ? std::nullopt + : std::optional{static_cast( + ptr - &this->container.nodes[0])}; + } + inline Self *r() { + return right.has_value() ? &container.nodes[right.value()] : nullptr; + } + inline void set_r(Self *ptr) { + this->right = ptr == nullptr + ? std::nullopt + : std::optional{static_cast( + ptr - &this->container.nodes[0])}; + } + inline Self *l() { + return left.has_value() ? &container.nodes[left.value()] : nullptr; + } + inline void set_l(Self *ptr) { + this->left = ptr == nullptr + ? std::nullopt + : std::optional{static_cast( + ptr - &this->container.nodes[0])}; + } + inline Self *par() { + return parent.has_value() ? &container.nodes[parent.value()] : nullptr; + } + inline void set_par(Self *ptr) { + this->parent = ptr == nullptr + ? std::nullopt + : std::optional{static_cast( + ptr - &this->container.nodes[0])}; + } // this is root/P for this method // copying from wikipedia RotateDirRoot with translation into my own idioms // https://en.wikipedia.org/wiki/Red%E2%80%93black_tree#Operations @@ -84,31 +147,30 @@ template struct BookKeeping { // wikipedia version uses alphabet soup, might fix later Self *P = this; auto &T = container; - Self *G = P->parent; + Self *G = P->par(); Self *S = P->child(!dir); Self *C; // this method shouldn't be called in cases where this assert will trip assert(S != nullptr); - // C = S->child(dir); - P->child(!dir) = C; + P->c_trans(!dir, C); if (C != nullptr) { - C->parent = P; + C->set_par(P); } - S->child(dir) = P; - P->parent = S; - S->parent = G; + S->c_trans(dir, P); + P->set_par(S); + S->set_par(G); if (G != nullptr) { - if (P == G->right) { - G->right = S; + if (P == G->r()) { + G->set_r(S); } else { - G->left = S; + G->set_l(S); } } else { - T.root = S; + T.root = S - &T.nodes[0]; } } }; @@ -134,22 +196,25 @@ public: friend Node; private: - // pointer needed so we can replace as needed - Node *ref; - Node *escape; - Iterator(Node *ref, Node *escape = nullptr) : ref{ref}, escape{escape} {} + using Ref_T = std::optional; + Map &parent; + Ref_T ref; + Ref_T escape; + Iterator(Map &parent, Ref_T ref, + Ref_T escape = std::nullopt) + : parent{parent}, ref{ref}, escape{escape} {} public: Iterator() = delete; Iterator &operator++() { - if (ref == nullptr) { + if (ref == std::nullopt) { ref = escape; return *this; } - if (ref->next == nullptr) { + if (parent.nodes[ref.value()].next == std::nullopt) { escape = ref; } - ref = ref->next; + ref = parent.nodes[ref.value()].next; return *this; } Iterator operator++(int) { @@ -158,14 +223,14 @@ public: return tmp; } Iterator &operator--() { - if (ref == nullptr) { + if (ref == std::nullopt) { ref = escape; return *this; } - if (ref->prev == nullptr) { + if (parent.nodes[ref.value()].prev == std::nullopt) { escape = ref; } - ref = ref->prev; + ref = parent.nodes[ref.value()].prev; return *this; } Iterator operator--(int) { @@ -173,8 +238,10 @@ public: --(*this); return tmp; } - ValueType &operator*() const { return this->ref->value; } - ValueType *operator->() const { return &this->ref->value; } + ValueType &operator*() const { + return this->parent.nodes[this->ref.value()].value; + } + ValueType *operator->() const { return &this->operator*(); } friend bool operator==(Iterator const &lhs, Iterator const &rhs) { return lhs.ref == rhs.ref; } @@ -277,58 +344,63 @@ public: }; private: - Node *root; - Node *min; - Node *max; + std::optional root; + std::optional min; + std::optional max; std::vector nodes; + std::size_t size_diff; public: - Map() : root{nullptr}, min{nullptr}, max{nullptr}, nodes{} {} + Map() + : root{std::nullopt}, min{std::nullopt}, max{std::nullopt}, nodes{}, + size_diff{0} {} Map(const Map &rhs) - : root{rhs.root}, min{nullptr}, max{nullptr}, nodes{rhs.nodes} {} + : root{rhs.root}, min{rhs.min}, max{rhs.max}, nodes{rhs.nodes}, + size_diff{0} {} Map &operator=(const Map &rhs) { - // TODO: fix this so all the pointers in all the bookkeeping nodes are - // updated in addition to root, min and max - assert(false); this->root = rhs.root; this->min = rhs.min; this->max = rhs.max; this->nodes = rhs.nodes; } - Map(std::initializer_list elems) : root{nullptr}, nodes{} { + Map(std::initializer_list elems) + : root{std::nullopt}, min{std::nullopt}, max{std::nullopt}, nodes{} { this->insert(elems.begin(), elems.end()); } ~Map() {} - size_t size() const { - root = nullptr; - return this->nodes.size(); - } + size_t size() const { return this->nodes.size() - this->size_diff; } bool empty() const { return this->size() == 0; } - Iterator begin() { return Iterator{min}; } - Iterator end() { return Iterator{nullptr, max}; } - ConstIterator begin() const { return ConstIterator{this->begin()}; } - ConstIterator end() const { return ConstIterator{this->end()}; } + + Iterator begin() { return Iterator{*this, min}; } + Iterator end() { return Iterator{*this, std::nullopt, max}; } + ConstIterator begin() const { return ConstIterator{*this, this->begin()}; } + ConstIterator end() const { return ConstIterator{*this, this->end()}; } ConstIterator cbegin() const { return this->begin(); } ConstIterator cend() const { return this->end(); } - ReverseIterator rbegin() { return ReverseIterator{Iterator{this->max}}; } - ReverseIterator rend() { return ReverseIterator{Iterator{nullptr, min}}; } + ReverseIterator rbegin() { + return ReverseIterator{Iterator{*this, this->max}}; + } + ReverseIterator rend() { + return ReverseIterator{Iterator{*this, nullptr, min}}; + } Iterator find(const Key_T &key) { // we need a locate slot function for insert regardless so might as well use // it here auto [parent, dir] = this->locate_slot(key); - if (parent == nullptr) { - if (this->root != nullptr && this->root->value.first == key) { - return Iterator{root}; + if (!parent.has_value()) { + if (this->root.has_value() && + this->nodes[this->root.value()].value.first == key) { + return Iterator{*this, root}; } else { return this->end(); } } - if (parent->child(dir) == nullptr) { + if (!this->nodes[parent.value()].c_select(dir).has_value()) { return this->end(); } - return Iterator{parent->child(dir)}; + return Iterator{*this, this->nodes[parent.value()].c_select(dir)}; } // implicit cast to ConstIterator from Iterator ConstIterator find(const Key_T &key) const { return this->find(key); } @@ -355,6 +427,14 @@ public: } private: + void handle_root_rotation(std::optional g, + std::optional p, + std::optional i, Direction dir) { + handle_root_rotation(g.has_value() ? &this->nodes[g.value()] : nullptr, + p.has_value() ? &this->nodes[p.value()] : nullptr, + i.has_value() ? &this->nodes[i.value()] : nullptr, + dir); + } void handle_root_rotation(Node *grandparent, Node *parent, Node *inserting, Direction dir) { // making inner grandchild into outer grandchild @@ -364,95 +444,101 @@ private: parent = grandparent->child(dir); } // RotateDirRoot(T,G,1-dir); - Node *gr_grandparent = grandparent->parent; + Node *gr_grandparent = grandparent->par(); Node *sibling = grandparent->child(!dir); assert(sibling != nullptr); Node *child = sibling->child(dir); - grandparent->child(!dir) = child; - sibling->child(dir) = grandparent; - grandparent->parent = sibling; - sibling->parent = gr_grandparent; + grandparent->c_trans(!dir, child); + sibling->c_trans(dir, grandparent); + grandparent->set_par(sibling); + sibling->set_par(gr_grandparent); if (gr_grandparent != nullptr) { Direction grandparent_direction; - if (gr_grandparent->left == grandparent) { + if (gr_grandparent->l() == grandparent) { grandparent_direction = Direction::Left; } else { grandparent_direction = Direction::Right; } - gr_grandparent->child(grandparent_direction) = sibling; + gr_grandparent->c_trans(grandparent_direction, sibling); } else { - this->root = sibling; + this->root = sibling - &this->nodes[0]; } parent->color = Color::Black; grandparent->color = Color::Red; } + using Ref_T = std::optional; // heavily referencing the wikipedia implementation for this // https://en.wikipedia.org/wiki/Red%E2%80%93black_tree#Insertion - void insert_helper(Node *to_insert, Node *parent, Direction dir) { + void insert_helper(Ref_T to_insert, Ref_T parent, Direction dir) { // initialize the element we're inserting - to_insert->color = Color::Red; - to_insert->left = nullptr; - to_insert->right = nullptr; - to_insert->parent = parent; - switch (dir) { - case Direction::Left: - to_insert->next = parent; - to_insert->prev = parent->prev; - parent->prev = to_insert; - parent->left = to_insert; - break; - case Direction::Right: - to_insert->prev = parent; - to_insert->next = parent->next; - parent->next = to_insert; - parent->right = to_insert; - break; - } + this->nodes[to_insert.value()].color = Color::Red; + this->nodes[to_insert.value()].left = std::nullopt; + this->nodes[to_insert.value()].right = std::nullopt; + this->nodes[to_insert.value()].next = std::nullopt; + this->nodes[to_insert.value()].prev = std::nullopt; + this->nodes[to_insert.value()].parent = parent; // if this is the first element to be inserted it's root - if (to_insert->parent == nullptr) { + if (!this->nodes[to_insert.value()].parent.has_value()) { this->root = to_insert; + this->nodes[to_insert.value()].color = Color::Black; return; } switch (dir) { case Direction::Left: - parent->left = to_insert; + this->nodes[to_insert.value()].next = parent; + this->nodes[to_insert.value()].prev = this->nodes[parent.value()].prev; + this->nodes[parent.value()].prev = to_insert; + this->nodes[parent.value()].left = to_insert; break; case Direction::Right: - parent->right = to_insert; + this->nodes[to_insert.value()].prev = parent; + this->nodes[to_insert.value()].next = this->nodes[parent.value()].next; + this->nodes[parent.value()].next = to_insert; + this->nodes[parent.value()].right = to_insert; + break; + } + + switch (dir) { + case Direction::Left: + this->nodes[parent.value()].left = to_insert; + break; + case Direction::Right: + this->nodes[parent.value()].right = to_insert; break; } do { // don't need to keep track of these in between loops they get // recalculated - Node *grandparent; - Node *uncle; - if (parent->color == Color::Black) { + std::optional grandparent; + std::optional uncle; + if (this->nodes[parent.value()].color == Color::Black) { // black parent means invariants definitely hold return; } - grandparent = parent->parent; + grandparent = this->nodes[parent.value()].parent; - if (grandparent == nullptr) { + if (!grandparent.has_value()) { // parent is root, just need to recolor it to black - parent->color = Color::Black; + this->nodes[parent.value()].color = Color::Black; return; } Direction parent_direction; - if (grandparent->left == parent) { + if (this->nodes[grandparent.value()].left == parent) { parent_direction = Direction::Left; - uncle = grandparent->right; + uncle = this->nodes[grandparent.value()].right; } else { parent_direction = Direction::Right; - uncle = grandparent->left; + uncle = this->nodes[grandparent.value()].left; } - if (uncle == nullptr || uncle->color == Color::Black) { + if (!uncle.has_value() || + this->nodes[uncle.value()].color == Color::Black) { // case 5 and 6 this->handle_root_rotation(grandparent, parent, to_insert, parent_direction); @@ -461,29 +547,32 @@ private: // now we know parent and uncle are both red so red-black coloring can be // pushed down from grandparent - parent->color = Color::Black; - uncle->color = Color::Black; - grandparent->color = Color::Red; + this->nodes[parent.value()].color = Color::Black; + this->nodes[uncle.value()].color = Color::Black; + this->nodes[grandparent.value()].color = Color::Red; to_insert = grandparent; - parent = to_insert->parent; - } while (parent != nullptr); + parent = this->nodes[to_insert.value()].parent; + } while (parent.has_value()); // case 3: current node is red root so we're done } // returns nullptr iff map is empty - std::pair locate_slot(const Key_T &key) { - Node *current = this->root; - Node *parent = nullptr; + std::pair, Direction> + locate_slot(const Key_T &key) { + using Ref_T = std::optional; + Ref_T current = this->root; + Ref_T parent = std::nullopt; Direction dir; - while (current != nullptr && current->value.first != key) { + while (current.has_value() && + this->nodes[current.value()].value.first != key) { parent = current; - if (key < current->value.first) { + if (key < this->nodes[current.value()].value.first) { dir = Direction::Left; - current = current->left; + current = this->nodes[current.value()].left; } else { dir = Direction::Right; - current = current->right; + current = this->nodes[current.value()].right; } } return std::make_pair(parent, dir); @@ -496,26 +585,30 @@ public: // an iterator pointing to the element with the same key, and false. std::pair insert(const ValueType &val) { auto [parent, dir] = locate_slot(val.first); - bool ret = parent == nullptr || parent->child(dir) == nullptr; + bool ret = !parent.has_value() || + !this->nodes[parent.value()].c_select(dir).has_value(); if (!ret) { - return std::make_pair(Iterator{parent->child(dir)}, ret); + return std::make_pair( + Iterator{*this, this->nodes[parent.value()].c_select(dir)}, ret); } Node to_insert{*this}; to_insert.value = val; this->nodes.push_back(std::move(to_insert)); // this->nodes.back().self = (--this->nodes.end()); - insert_helper(&nodes.back(), parent, dir); + insert_helper(nodes.size() - 1, parent, dir); - if (min == nullptr || val.first < min->value.first) { - min = &nodes.back(); - nodes.back().prev = nullptr; + if (min == std::nullopt || + val.first < this->nodes[min.value()].value.first) { + min = nodes.size() - 1; + nodes.back().prev = std::nullopt; } - if (max == nullptr || val.first > max->value.first) { - max = &nodes.back(); - nodes.back().next = nullptr; + if (max == std::nullopt || + val.first > this->nodes[max.value()].value.first) { + max = nodes.size() - 1; + nodes.back().next = std::nullopt; } - return std::make_pair(Iterator(&nodes.back()), ret); + return std::make_pair(Iterator(*this, nodes.size() - 1), ret); } template void insert(IT_T range_beg, IT_T range_end) { std::for_each(range_beg, range_end, @@ -542,22 +635,22 @@ private: // https://en.wikipedia.org/wiki/Red%E2%80%93black_tree#Removal_of_a_black_non-root_leaf void complex_erase(Iterator pos) { - Node *to_delete = pos.ref; - Node *parent = to_delete->parent; + Node *to_delete = &this->nodes[pos.ref.value()]; + Node *parent = to_delete->par(); assert(parent != nullptr); Direction dir = - parent->right == to_delete ? Direction::Right : Direction::Left; + parent->r() == to_delete ? Direction::Right : Direction::Left; Node *sibling; ; Node *close_nephew; Node *distant_nephew; - parent->child(dir) = nullptr; + parent->c_trans(dir, nullptr); do { - dir = parent->right == to_delete ? Direction::Right : Direction::Left; + dir = parent->r() == to_delete ? Direction::Right : Direction::Left; sibling = parent->child(!dir); distant_nephew = sibling->child(!dir); @@ -605,56 +698,67 @@ private: // case 2 sibling->color = Color::Red; to_delete = parent; - parent = to_delete->parent; + parent = to_delete->par(); } while (parent != nullptr); } public: // TODO: check that the way of reconnecting next and prev works void erase(Iterator pos) { + this->size_diff++; // simple cases - Node *ref = pos.ref; - if (ref == this->min) { + Node *ref = &this->nodes[pos.ref.value()]; + if (ref == + (this->min.has_value() ? &this->nodes[this->min.value()] : nullptr)) { this->min = ref->next; } - if (ref == this->max) { + if (ref == + (this->max.has_value() ? &this->nodes[this->max.value()] : nullptr)) { this->max = ref->prev; } // 2 children - if (ref->left != nullptr && ref->right != nullptr) { - Node *next = ref->next; - Node *prev = ref->prev; - *ref = *next; - prev->next = next; - next->prev = prev; - this->erase(Iterator{next}); + if (ref->l() != nullptr && ref->r() != nullptr) { + Ref_T next = ref->next; + Ref_T prev = ref->prev; + *ref = this->nodes[next.value()]; + this->nodes[prev.value()].next = next; + this->nodes[next.value()].prev = prev; + this->erase(Iterator{*this, next}); } // single child which is left - else if (ref->left != nullptr && ref->right == nullptr) { - Node *next = ref->next; - Node *prev = ref->prev; - *ref = *ref->left; - prev->next = next; - next->prev = prev; + else if (ref->l() != nullptr && ref->r() == nullptr) { + Ref_T next = ref->next; + Ref_T prev = ref->prev; + *ref = *ref->l(); + this->nodes[prev.value()].next = next; + this->nodes[next.value()].prev = prev; } // single child which is right - else if (ref->left == nullptr && ref->right != nullptr) { - Node *next = ref->next; - Node *prev = ref->prev; - *ref = *ref->right; - prev->next = next; - next->prev = prev; + else if (ref->l() == nullptr && ref->r() != nullptr) { + Ref_T next = ref->next; + Ref_T prev = ref->prev; + *ref = *ref->r(); + if (prev.has_value()) { + this->nodes[prev.value()].next = next; + } + if (next.has_value()) { + this->nodes[next.value()].prev = prev; + } } // no children and root - else if (ref->left == nullptr && ref->right == nullptr) { - this->root = nullptr; + else if (ref->l() == nullptr && ref->r() == nullptr) { + this->root = std::nullopt; } // no children and red - else if (ref->left == nullptr && ref->right == nullptr) { - Node *next = ref->next; - Node *prev = ref->prev; - prev->next = next; - next->prev = prev; + else if (ref->l() == nullptr && ref->r() == nullptr) { + Node *next = + ref->next.has_value() ? &this->nodes[ref->next.value()] : nullptr; + Node *prev = + ref->prev.has_value() ? &this->nodes[ref->prev.value()] : nullptr; + prev->next = next != nullptr ? std::optional{next - &this->nodes[0]} + : std::nullopt; + next->prev = prev != nullptr ? std::optional{prev - &this->nodes[0]} + : std::nullopt; } // complicated case of black node with no kids else { @@ -663,7 +767,7 @@ public: } void erase(const Key_T &key) { this->erase(this->find(key)); } void clear() { - this->root = nullptr; + this->root = std::nullopt; this->nodes.clear(); } friend bool operator==(const Map &lhs, const Map &rhs) {