From 27409cc0b0af47b54ccaf2e922a6e776e928c783 Mon Sep 17 00:00:00 2001 From: Pagwin Date: Sun, 24 Nov 2024 15:20:10 -0500 Subject: [PATCH] fixed memory issues, and bugs in insertion and deletion, added checks on the way --- Map.hpp | 54 +++++++++++++++++++++++++++++++++++++++++++----------- suspects | 16 +--------------- t.cpp | 24 ++++++++++++------------ 3 files changed, 56 insertions(+), 38 deletions(-) diff --git a/Map.hpp b/Map.hpp index 42a368c..37a1a53 100644 --- a/Map.hpp +++ b/Map.hpp @@ -58,10 +58,14 @@ template class Map { : parent{nullptr}, val{std::move(rhs.val)}, left{std::move(rhs.left)}, right{std::move(rhs.right)}, color{rhs.color}, prev{nullptr}, next{nullptr}, map{rhs.map} { + // TODO: remove on finish + if (rhs.valid != 0x13371337) { std::cerr << "(" << rhs.val.first << ")" << std::endl; } rhs.valid = 0; + this->valid = 0x13371337; + if (this->left) { this->left->parent = this; } @@ -78,6 +82,7 @@ template class Map { this->left = std::make_unique(*rhs.left); this->right = std::make_unique(*rhs.right); this->color = rhs.color; + this->valid = 0x13371337; if (this->left) { this->left->parent = this; this->left->restore_ordering(); @@ -98,6 +103,8 @@ template class Map { this->left = std::move(rhs.left); this->right = std::move(rhs.right); this->color = rhs.color; + this->valid = 0x13371337; + rhs.valid = 0; if (this->left) { this->left->parent = this; this->left->restore_ordering(); @@ -274,6 +281,7 @@ template class Map { Node *parent = self->parent; // we're root, no-op (case 3) if (!parent) { + self->color = Color::Black; return; } @@ -347,12 +355,29 @@ public: friend Map; Iterator() = delete; void check() { - assert(underlying->val.first < 200); - if (underlying->prev != nullptr) { - assert(underlying->prev->val.first < 200); + if (this->underlying == nullptr) { + return; } - if (underlying->next != nullptr) { - assert(underlying->next->val.first < 200); + if (this->underlying->parent) { + switch (this->underlying->parent->which_child(this->underlying)) { + case Direction::Left: + assert(this->underlying->val.first < + this->underlying->parent->val.first); + break; + case Direction::Right: + assert(this->underlying->val.first > + this->underlying->parent->val.first); + break; + default: + assert(false); + } + } + if (this->underlying->right) { + assert(this->underlying->right->val.first > + this->underlying->val.first); + } + if (this->underlying->left) { + assert(this->underlying->left->val.first < this->underlying->val.first); } } }; @@ -369,6 +394,9 @@ public: this->_size = rhs._size; return *this; } + void check() { + assert(!this->root || this->root.value().color == Color::Black); + } std::size_t size() { return this->_size; } private: @@ -383,9 +411,9 @@ private: std::unique_ptr old_root = std::make_unique(std::move(root.value())); - root.value() = std::move(*new_root); + root.value() = std::move(*new_root.release()); - old_root->set_child(!dir, old_root->uchild(dir)); + old_root->set_child(!dir, root.value().uchild(dir)); if (old_root->left) { old_root->left->parent = old_root.get(); @@ -554,6 +582,7 @@ private: } } bool core_erase(Node *erasing) { + Color c = erasing->color; // 2 children if (erasing->left && erasing->right) { Node *succ = erasing->next; @@ -562,27 +591,28 @@ private: } // 1 child else if (erasing->left) { - *erasing = std::move(*erasing->left); + *erasing = std::move(*erasing->left.release()); if (erasing->prev != nullptr) { erasing->prev->next = erasing; } if (erasing->next != nullptr) { erasing->next->prev = erasing; } + erasing->color = c; return true; } else if (erasing->right) { - //!! - *erasing = std::move(*erasing->right); + *erasing = std::move(*erasing->right.release()); if (erasing->prev != nullptr) { erasing->prev->next = erasing; } if (erasing->next != nullptr) { erasing->next->prev = erasing; } + erasing->color = c; return true; } // no children and root - else if (!erasing->parent) { + else if (erasing->parent == nullptr) { erasing->map->root = std::nullopt; } // no children and red @@ -640,6 +670,7 @@ public: return std::make_pair(Iterator{&root.value()}, false); } else { this->root = Node{val, this}; + this->root.value().color = Color::Black; return std::make_pair(Iterator{&root.value()}, true); } } @@ -654,6 +685,7 @@ public: Node *new_node = parent->set_child(dir, std::make_unique(Node{val, this})).get(); new_node->restore_red_black_insert(dir); + new_node->restore_ordering(); return std::make_pair(Iterator{new_node}, true); } diff --git a/suspects b/suspects index 799d4c8..8bd2efe 100644 --- a/suspects +++ b/suspects @@ -1,15 +1 @@ -suspects: - -PARENT is definitely problematic somewhere, next and prev being removed doesn't solve the problem - -core_erase - -erase_child - -hard_erase - -rotate - -Soft okay: - -assignment operator for node specifically dealing with the parent pointer if we assign to something and then don't immediately set the parent appropriately bad things happen, just setting to rhs didn't fix but it also didn't break anything so it's soft okay +rotation is definitely wrong somewhere, knowing me rotate_root, somehow 1 ended up right of 2 diff --git a/t.cpp b/t.cpp index 099a42c..8e5eb47 100644 --- a/t.cpp +++ b/t.cpp @@ -3,20 +3,20 @@ template class cs440::Map; int main(void) { cs440::Map a; - a.insert({1, 1}); - a.insert({2, 2}); - a.insert({3, 3}); - a.insert({4, 4}); - a.insert({5, 5}); - a.insert({6, 6}); - a.insert({7, 7}); - a.insert({8, 8}); - a.insert({9, 9}); - a.insert({10, 10}); for (std::size_t i = 1; i <= 10; i++) { - // a.find(i).check(); + a.insert({i, i}); + a.check(); + for (std::size_t j = 1; j <= i; j++) { + a.find(j).check(); + } + } + for (std::size_t i = 1; i <= 10; i++) { + a.find(i).check(); + } + for (std::size_t i = 1; i <= 5; i++) { std::cout << i << std::endl; - a.erase(a.find(i)); + auto b = a.find(i); + a.erase(b); } return 0; }