fixed memory issues, and bugs in insertion and deletion, added checks on the way
This commit is contained in:
parent
9cdecb790b
commit
27409cc0b0
3 changed files with 56 additions and 38 deletions
54
Map.hpp
54
Map.hpp
|
@ -58,10 +58,14 @@ template <typename Key_T, typename Mapped_T> class Map {
|
||||||
: parent{nullptr}, val{std::move(rhs.val)}, left{std::move(rhs.left)},
|
: parent{nullptr}, val{std::move(rhs.val)}, left{std::move(rhs.left)},
|
||||||
right{std::move(rhs.right)}, color{rhs.color}, prev{nullptr},
|
right{std::move(rhs.right)}, color{rhs.color}, prev{nullptr},
|
||||||
next{nullptr}, map{rhs.map} {
|
next{nullptr}, map{rhs.map} {
|
||||||
|
// TODO: remove on finish
|
||||||
|
|
||||||
if (rhs.valid != 0x13371337) {
|
if (rhs.valid != 0x13371337) {
|
||||||
std::cerr << "(" << rhs.val.first << ")" << std::endl;
|
std::cerr << "(" << rhs.val.first << ")" << std::endl;
|
||||||
}
|
}
|
||||||
rhs.valid = 0;
|
rhs.valid = 0;
|
||||||
|
this->valid = 0x13371337;
|
||||||
|
|
||||||
if (this->left) {
|
if (this->left) {
|
||||||
this->left->parent = this;
|
this->left->parent = this;
|
||||||
}
|
}
|
||||||
|
@ -78,6 +82,7 @@ template <typename Key_T, typename Mapped_T> class Map {
|
||||||
this->left = std::make_unique<Node>(*rhs.left);
|
this->left = std::make_unique<Node>(*rhs.left);
|
||||||
this->right = std::make_unique<Node>(*rhs.right);
|
this->right = std::make_unique<Node>(*rhs.right);
|
||||||
this->color = rhs.color;
|
this->color = rhs.color;
|
||||||
|
this->valid = 0x13371337;
|
||||||
if (this->left) {
|
if (this->left) {
|
||||||
this->left->parent = this;
|
this->left->parent = this;
|
||||||
this->left->restore_ordering();
|
this->left->restore_ordering();
|
||||||
|
@ -98,6 +103,8 @@ template <typename Key_T, typename Mapped_T> class Map {
|
||||||
this->left = std::move(rhs.left);
|
this->left = std::move(rhs.left);
|
||||||
this->right = std::move(rhs.right);
|
this->right = std::move(rhs.right);
|
||||||
this->color = rhs.color;
|
this->color = rhs.color;
|
||||||
|
this->valid = 0x13371337;
|
||||||
|
rhs.valid = 0;
|
||||||
if (this->left) {
|
if (this->left) {
|
||||||
this->left->parent = this;
|
this->left->parent = this;
|
||||||
this->left->restore_ordering();
|
this->left->restore_ordering();
|
||||||
|
@ -274,6 +281,7 @@ template <typename Key_T, typename Mapped_T> class Map {
|
||||||
Node *parent = self->parent;
|
Node *parent = self->parent;
|
||||||
// we're root, no-op (case 3)
|
// we're root, no-op (case 3)
|
||||||
if (!parent) {
|
if (!parent) {
|
||||||
|
self->color = Color::Black;
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -347,12 +355,29 @@ public:
|
||||||
friend Map;
|
friend Map;
|
||||||
Iterator() = delete;
|
Iterator() = delete;
|
||||||
void check() {
|
void check() {
|
||||||
assert(underlying->val.first < 200);
|
if (this->underlying == nullptr) {
|
||||||
if (underlying->prev != nullptr) {
|
return;
|
||||||
assert(underlying->prev->val.first < 200);
|
|
||||||
}
|
}
|
||||||
if (underlying->next != nullptr) {
|
if (this->underlying->parent) {
|
||||||
assert(underlying->next->val.first < 200);
|
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;
|
this->_size = rhs._size;
|
||||||
return *this;
|
return *this;
|
||||||
}
|
}
|
||||||
|
void check() {
|
||||||
|
assert(!this->root || this->root.value().color == Color::Black);
|
||||||
|
}
|
||||||
std::size_t size() { return this->_size; }
|
std::size_t size() { return this->_size; }
|
||||||
|
|
||||||
private:
|
private:
|
||||||
|
@ -383,9 +411,9 @@ private:
|
||||||
std::unique_ptr<Node> old_root =
|
std::unique_ptr<Node> old_root =
|
||||||
std::make_unique<Node>(std::move(root.value()));
|
std::make_unique<Node>(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) {
|
if (old_root->left) {
|
||||||
old_root->left->parent = old_root.get();
|
old_root->left->parent = old_root.get();
|
||||||
|
@ -554,6 +582,7 @@ private:
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
bool core_erase(Node *erasing) {
|
bool core_erase(Node *erasing) {
|
||||||
|
Color c = erasing->color;
|
||||||
// 2 children
|
// 2 children
|
||||||
if (erasing->left && erasing->right) {
|
if (erasing->left && erasing->right) {
|
||||||
Node *succ = erasing->next;
|
Node *succ = erasing->next;
|
||||||
|
@ -562,27 +591,28 @@ private:
|
||||||
}
|
}
|
||||||
// 1 child
|
// 1 child
|
||||||
else if (erasing->left) {
|
else if (erasing->left) {
|
||||||
*erasing = std::move(*erasing->left);
|
*erasing = std::move(*erasing->left.release());
|
||||||
if (erasing->prev != nullptr) {
|
if (erasing->prev != nullptr) {
|
||||||
erasing->prev->next = erasing;
|
erasing->prev->next = erasing;
|
||||||
}
|
}
|
||||||
if (erasing->next != nullptr) {
|
if (erasing->next != nullptr) {
|
||||||
erasing->next->prev = erasing;
|
erasing->next->prev = erasing;
|
||||||
}
|
}
|
||||||
|
erasing->color = c;
|
||||||
return true;
|
return true;
|
||||||
} else if (erasing->right) {
|
} else if (erasing->right) {
|
||||||
//!!
|
*erasing = std::move(*erasing->right.release());
|
||||||
*erasing = std::move(*erasing->right);
|
|
||||||
if (erasing->prev != nullptr) {
|
if (erasing->prev != nullptr) {
|
||||||
erasing->prev->next = erasing;
|
erasing->prev->next = erasing;
|
||||||
}
|
}
|
||||||
if (erasing->next != nullptr) {
|
if (erasing->next != nullptr) {
|
||||||
erasing->next->prev = erasing;
|
erasing->next->prev = erasing;
|
||||||
}
|
}
|
||||||
|
erasing->color = c;
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
// no children and root
|
// no children and root
|
||||||
else if (!erasing->parent) {
|
else if (erasing->parent == nullptr) {
|
||||||
erasing->map->root = std::nullopt;
|
erasing->map->root = std::nullopt;
|
||||||
}
|
}
|
||||||
// no children and red
|
// no children and red
|
||||||
|
@ -640,6 +670,7 @@ public:
|
||||||
return std::make_pair(Iterator{&root.value()}, false);
|
return std::make_pair(Iterator{&root.value()}, false);
|
||||||
} else {
|
} else {
|
||||||
this->root = Node{val, this};
|
this->root = Node{val, this};
|
||||||
|
this->root.value().color = Color::Black;
|
||||||
return std::make_pair(Iterator{&root.value()}, true);
|
return std::make_pair(Iterator{&root.value()}, true);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -654,6 +685,7 @@ public:
|
||||||
Node *new_node =
|
Node *new_node =
|
||||||
parent->set_child(dir, std::make_unique<Node>(Node{val, this})).get();
|
parent->set_child(dir, std::make_unique<Node>(Node{val, this})).get();
|
||||||
new_node->restore_red_black_insert(dir);
|
new_node->restore_red_black_insert(dir);
|
||||||
|
|
||||||
new_node->restore_ordering();
|
new_node->restore_ordering();
|
||||||
return std::make_pair(Iterator{new_node}, true);
|
return std::make_pair(Iterator{new_node}, true);
|
||||||
}
|
}
|
||||||
|
|
16
suspects
16
suspects
|
@ -1,15 +1 @@
|
||||||
suspects:
|
rotation is definitely wrong somewhere, knowing me rotate_root, somehow 1 ended up right of 2
|
||||||
|
|
||||||
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
|
|
||||||
|
|
24
t.cpp
24
t.cpp
|
@ -3,20 +3,20 @@
|
||||||
template class cs440::Map<int, int>;
|
template class cs440::Map<int, int>;
|
||||||
int main(void) {
|
int main(void) {
|
||||||
cs440::Map<int, int> a;
|
cs440::Map<int, int> 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++) {
|
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;
|
std::cout << i << std::endl;
|
||||||
a.erase(a.find(i));
|
auto b = a.find(i);
|
||||||
|
a.erase(b);
|
||||||
}
|
}
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue