fix(heap): finally fixed that pop bug

src/heap/vieter_heap.c

@@ -1,12 +1,7 @@
-#include "vieter_heap.h"
-#include "vieter_heap_tree.h"
+#include "vieter_heap_internal.h"
 
 #include <stdlib.h>
 
-struct vieter_heap {
-  vieter_heap_node *tree;
-};
-
 vieter_heap *vieter_heap_init() { return calloc(1, sizeof(vieter_heap)); }
 
 uint64_t vieter_heap_size(vieter_heap *heap) {

src/heap/vieter_heap_internal.h

@@ -0,0 +1,6 @@
+#include "vieter_heap.h"
+#include "vieter_heap_tree.h"
+
+struct vieter_heap {
+  vieter_heap_node *tree;
+};

src/heap/vieter_heap_tree.c

@@ -43,90 +43,96 @@ end:
 
 vieter_heap_node *vieter_heap_tree_merge_same_order(vieter_heap_node *root_a,
                                                     vieter_heap_node *root_b) {
-  vieter_heap_node *new_root;
+  vieter_heap_node *root, *child;
 
   if (root_a->key <= root_b->key) {
-    new_root = root_a;
-    root_b->ptr.next_largest_order = root_a->largest_order;
-    root_a->largest_order = root_b;
+    root = root_a;
+    child = root_b;
   } else {
-    new_root = root_b;
-    root_a->ptr.next_largest_order = root_b->largest_order;
-    root_b->largest_order = root_a;
+    root = root_b;
+    child = root_a;
   }
 
-  new_root->order++;
+  child->ptr.next_largest_order = root->largest_order;
+  root->largest_order = child;
 
-  return new_root;
+  root->order++;
+
+  return root;
 }
 
-vieter_heap_node *vieter_heap_tree_merge(vieter_heap_node *root_a,
-                                         vieter_heap_node *root_b) {
-  vieter_heap_node *root, *target, *out;
+vieter_heap_node *vieter_heap_tree_merge(vieter_heap_node *target_tree,
+                                         vieter_heap_node *other_tree) {
+  /* vieter_heap_node *other_tree, *target_tree, *out; */
+  vieter_heap_node *out;
 
-  if (root_a->order <= root_b->order) {
-    target = root_a;
-    root = root_b;
-  } else {
-    target = root_b;
-    root = root_a;
-  }
+  /* if (root_a->order <= root_b->order) { */
+  /*   target_tree = root_a; */
+  /*   other_tree = root_b; */
+  /* } else { */
+  /*   target_tree = root_b; */
+  /*   other_tree = root_a; */
+  /* } */
 
-  vieter_heap_node *next_tree, *next_target;
-  vieter_heap_node *previous_target = NULL;
+  vieter_heap_node *next_other_tree, *next_target_tree;
+  vieter_heap_node *previous_target_tree = NULL;
 
-  while (target != NULL && root != NULL) {
-    if (target->order == root->order) {
-      next_tree = root->ptr.next_tree;
-      next_target = target->ptr.next_tree;
+  while (target_tree != NULL && other_tree != NULL) {
+    if (target_tree->order == other_tree->order) {
+      next_other_tree = other_tree->ptr.next_tree;
+      next_target_tree = target_tree->ptr.next_tree;
 
-      target = vieter_heap_tree_merge_same_order(target, root);
+      target_tree = vieter_heap_tree_merge_same_order(target_tree, other_tree);
 
-      target->ptr.next_tree = next_target;
+      target_tree->ptr.next_tree = next_target_tree;
 
       // If this merge produces a binomial tree whose size is already in
       // target, it will be the next target. Therefore, we can  merge target's
       // trees until we no longer have a duplicate depth.
-      while (target->ptr.next_tree != NULL &&
-             target->ptr.next_tree->order == target->order) {
-        next_target = target->ptr.next_tree->ptr.next_tree;
-        target =
-            vieter_heap_tree_merge_same_order(target, target->ptr.next_tree);
-        target->ptr.next_tree = next_target;
+      while (next_target_tree != NULL &&
+             next_target_tree->order == target_tree->order) {
+        next_target_tree = next_target_tree->ptr.next_tree;
+        target_tree = vieter_heap_tree_merge_same_order(
+            target_tree, target_tree->ptr.next_tree);
+        target_tree->ptr.next_tree = next_target_tree;
       }
 
-      if (previous_target != NULL) {
-        previous_target->ptr.next_tree = target;
+      if (previous_target_tree != NULL) {
+        previous_target_tree->ptr.next_tree = target_tree;
       } else {
-        out = target;
+        out = target_tree;
       }
 
-      root = next_tree;
-    } else if (target->order > root->order) {
-      next_tree = root->ptr.next_tree;
+      other_tree = next_other_tree;
+    } else if (target_tree->order > other_tree->order) {
+      next_other_tree = other_tree->ptr.next_tree;
 
-      if (previous_target == NULL) {
-        previous_target = root;
-        out = root;
+      if (previous_target_tree == NULL) {
+        previous_target_tree = other_tree;
+        out = other_tree;
       } else {
-        previous_target->ptr.next_tree = root;
+        previous_target_tree->ptr.next_tree = other_tree;
+
+        // This single missing line right here broke this entire function for
+        // nearly a week.
+        previous_target_tree = other_tree;
       }
 
-      root->ptr.next_tree = target;
-      root = next_tree;
+      other_tree->ptr.next_tree = target_tree;
+      other_tree = next_other_tree;
     } else {
-      if (previous_target == NULL) {
-        out = target;
+      if (previous_target_tree == NULL) {
+        out = target_tree;
       }
 
-      previous_target = target;
-      target = target->ptr.next_tree;
+      previous_target_tree = target_tree;
+      target_tree = target_tree->ptr.next_tree;
     }
   }
 
   // Append final part of tree to target
-  if (target == NULL) {
-    previous_target->ptr.next_tree = root;
+  if (target_tree == NULL) {
+    previous_target_tree->ptr.next_tree = other_tree;
   }
 
   return out;

test/heap/test_heap.c

@@ -1,6 +1,5 @@
 #include "acutest.h"
-#include "vieter_heap.h"
-#include "vieter_heap_tree.h"
+#include "vieter_heap_internal.h"
 #include <stdlib.h>
 
 #define TEST_SIZE(heap, size) \
@@ -14,6 +13,29 @@ void test_init() {
     vieter_heap_free(heap);
 }
 
+void count_nodes(uint64_t *counter, vieter_heap_node *root) {
+    (*counter)++;
+
+    if (root->largest_order != NULL) {
+        count_nodes(counter, root->largest_order);
+    }
+
+    // This will also traverse the various trees
+    if (root->ptr.next_largest_order != NULL) {
+        count_nodes(counter, root->ptr.next_largest_order);
+    }
+}
+
+uint64_t count_nodes_heap(vieter_heap *heap) {
+    uint64_t counter = 0;
+
+    if (heap->tree != NULL) {
+        count_nodes(&counter, heap->tree);
+    }
+
+    return counter;
+}
+
 void test_insert() {
     vieter_heap *heap = vieter_heap_init();
     TEST_SIZE(heap, 0);
@@ -23,6 +45,7 @@ void test_insert() {
     for (uint64_t i = 50; i > 0; i--) {
         vieter_heap_insert(heap, i, (void *)i);
         TEST_SIZE(heap, (uint64_t)51 - i);
+        TEST_CHECK(count_nodes_heap(heap) == (uint64_t)51 - i);
 
         data = 0;
 
@@ -47,6 +70,7 @@ void test_insert_random() {
     for (uint64_t i = 0; i < 5000; i++) {
         vieter_heap_insert(heap, num, (void *)num);
         TEST_SIZE(heap, i + 1);
+        TEST_CHECK(count_nodes_heap(heap) == (uint64_t)i + 1);
 
         if (num < smallest) {
             smallest = num;
@@ -64,14 +88,17 @@ void test_insert_random() {
 }
 
 void test_pop() {
+    const uint64_t n = 500;
+
     vieter_heap *heap = vieter_heap_init();
     TEST_SIZE(heap, 0);
 
     void *data;
 
-    for (uint64_t i = 50; i > 0; i--) {
+    for (uint64_t i = n; i > 0; i--) {
         vieter_heap_insert(heap, i, (void *)i);
-        TEST_SIZE(heap, (uint64_t)51 - i);
+        TEST_SIZE(heap, (uint64_t)n + 1 - i);
+        TEST_CHECK(count_nodes_heap(heap) == (uint64_t)n + 1 - i);
 
         TEST_CHECK(vieter_heap_peek(&data, heap) == vieter_heap_ok);
         TEST_CHECK(data == (void*)i);
@@ -79,10 +106,10 @@ void test_pop() {
 
     data = NULL;
 
-    for (uint64_t i = 1; i <= 50; i++) {
+    for (uint64_t i = 1; i <= n; i++) {
         TEST_CHECK(vieter_heap_pop(&data, heap) == vieter_heap_ok);
         TEST_CHECK(data == (void*)i);
-        TEST_SIZE(heap, (uint64_t)50 - i);
+        TEST_SIZE(heap, (uint64_t)n - i);
     }
 
     vieter_heap_free(heap);
@@ -99,7 +126,7 @@ int uint64_t_compare(const void *a, const void *b) {
 }
 
 void test_pop_random() {
-    const uint64_t n = 29;
+    const uint64_t n = 500;
 
     srand(0);
 
@@ -112,6 +139,7 @@ void test_pop_random() {
         num = rand();
         vieter_heap_insert(heap, num, (void *)num);
         TEST_SIZE(heap, i + 1);
+        TEST_CHECK(count_nodes_heap(heap) == i + 1);
 
         numbers[i] = num;
     }
@@ -130,6 +158,7 @@ void test_pop_random() {
         TEST_CHECK(vieter_heap_pop(&data, heap) == vieter_heap_ok);
         TEST_CHECK_(data == (void *)numbers[i], "pop %lx == %lx", (uint64_t)data, numbers[i]);
         TEST_SIZE(heap, n - i - 1);
+        TEST_CHECK(count_nodes_heap(heap) == n - i - 1);
     }
 
     vieter_heap_free(heap);