Remove some redundant nullptr checks

and remove resulting dead code

ConflictSet.cpp

@@ -1672,7 +1672,10 @@ Vector<uint8_t> getSearchPath(Arena &arena, Node *n) {
 } // GCOVR_EXCL_LINE
 
 // Return true if the max version among all keys that start with key + [child],
-// where begin < child < end, is <= readVersion
+// where begin < child < end, is <= readVersion.
+//
+// Precondition: transitively, no child of n has a search path that's a longer
+// prefix of key than n
 bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
                           int end, int64_t readVersion,
                           ConflictSet::Impl *impl) {
@@ -1680,56 +1683,51 @@ bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
   fprintf(stderr, "%s(%02x,%02x)*\n", printable(key).c_str(), begin, end);
 #endif
   auto remaining = key;
-  for (;;) {
-    if (maxVersion(n, impl) <= readVersion) {
-      return true;
-    }
-    if (remaining.size() == 0) {
-      return maxBetweenExclusive(n, begin, end) <= readVersion;
-    }
+  if (remaining.size() == 0) {
+    return maxBetweenExclusive(n, begin, end) <= readVersion;
+  }
 
-    auto *child = getChild(n, remaining[0]);
-    if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
-        n = getChildExists(n, c);
+  auto *child = getChild(n, remaining[0]);
+  if (child == nullptr) {
+    int c = getChildGeq(n, remaining[0]);
+    if (c >= 0) {
+      n = getChildExists(n, c);
+      goto downLeftSpine;
+    } else {
+      n = nextSibling(n);
+      goto downLeftSpine;
+    }
+  }
+
+  n = child;
+  remaining = remaining.subspan(1, remaining.size() - 1);
+
+  assert(n->partialKeyLen > 0);
+  {
+    int commonLen = std::min<int>(n->partialKeyLen, remaining.size());
+    int i = longestCommonPrefix(n->partialKey(), remaining.data(), commonLen);
+    if (i < commonLen) {
+      auto c = n->partialKey()[i] <=> remaining[i];
+      if (c > 0) {
         goto downLeftSpine;
       } else {
         n = nextSibling(n);
         goto downLeftSpine;
       }
     }
-
-    n = child;
-    remaining = remaining.subspan(1, remaining.size() - 1);
-
-    if (n->partialKeyLen > 0) {
-      int commonLen = std::min<int>(n->partialKeyLen, remaining.size());
-      int i = longestCommonPrefix(n->partialKey(), remaining.data(), commonLen);
-      if (i < commonLen) {
-        auto c = n->partialKey()[i] <=> remaining[i];
-        if (c > 0) {
-          goto downLeftSpine;
-        } else {
-          n = nextSibling(n);
-          goto downLeftSpine;
-        }
-      }
-      if (commonLen == n->partialKeyLen) {
-        // partial key matches
-        remaining = remaining.subspan(commonLen, remaining.size() - commonLen);
-      } else if (n->partialKeyLen > int(remaining.size())) {
-        if (begin < n->partialKey()[remaining.size()] &&
-            n->partialKey()[remaining.size()] < end) {
-          if (n->entryPresent && n->entry.rangeVersion > readVersion) {
-            return false;
-          }
-          return maxVersion(n, impl) <= readVersion;
-        }
-        return true;
+    assert(n->partialKeyLen > int(remaining.size()));
+    if (begin < n->partialKey()[remaining.size()] &&
+        n->partialKey()[remaining.size()] < end) {
+      if (n->entryPresent && n->entry.rangeVersion > readVersion) {
+        return false;
       }
+      return maxVersion(n, impl) <= readVersion;
     }
+    return true;
   }
+
+  __builtin_unreachable(); // GCOVR_EXCL_LINE
+
 downLeftSpine:
   if (n == nullptr) {
     return true;
@@ -1836,9 +1834,7 @@ struct CheckRangeLeftSide {
           remaining =
               remaining.subspan(commonLen, remaining.size() - commonLen);
         } else if (n->partialKeyLen > int(remaining.size())) {
-          if (searchPathLen < prefixLen) {
-            return downLeftSpine();
-          }
+          assert(searchPathLen >= prefixLen);
           if (n->entryPresent && n->entry.rangeVersion > readVersion) {
             ok = false;
             return true;
@@ -2017,10 +2013,7 @@ struct CheckRangeRightSide {
 
   bool downLeftSpine() {
     phase = DownLeftSpine;
-    if (n == nullptr) {
-      ok = true;
-      return true;
-    }
+    assert(n != nullptr);
     return false;
   }
 };
@@ -2356,6 +2349,9 @@ Iterator firstGeq(Node *n, const std::span<const uint8_t> key) {
         goto downLeftSpine;
       } else {
         n = nextSibling(n);
+        if (n == nullptr) {
+          return {nullptr, 1};
+        }
         goto downLeftSpine;
       }
     }
@@ -2386,9 +2382,6 @@ Iterator firstGeq(Node *n, const std::span<const uint8_t> key) {
     }
   }
 downLeftSpine:
-  if (n == nullptr) {
-    return {nullptr, 1};
-  }
   for (;;) {
     if (n->entryPresent) {
       return {n, 1};
@@ -2438,9 +2431,14 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
       return;
     }
     this->oldestVersion = oldestVersion;
+#ifdef NDEBUG
+    // This is here for performance reasons, since we want to amortize the cost
+    // of storing the search path as a string. In tests, we want to exercise the
+    // rest of the code often.
     if (keyUpdates < 100) {
       return;
     }
+#endif
     Node *n = firstGeq(root, removalKey).n;
     // There's no way to erase removalKey without introducing a key after it
     assert(n != nullptr);
@@ -2468,9 +2466,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
       removalKey = {};
       return;
     }
-    if (keyUpdates == 0) {
-      keyUpdates = 10;
-    }
     removalKeyArena = Arena();
     removalKey = getSearchPath(removalKeyArena, n);
   }
@@ -2511,7 +2506,7 @@ int64_t &maxVersion(Node *n, ConflictSet::Impl *impl) {
     return impl->rootMaxVersion;
   }
   switch (n->getType()) {
-  case Type_Node0:
+  case Type_Node0:           // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   case Type_Node3: {
     auto *n3 = static_cast<Node3 *>(n);