[libc-commits] [libc] libcBlockstoreErase (PR #98674)
Michael Jones via libc-commits
libc-commits at lists.llvm.org
Fri Jul 12 11:16:26 PDT 2024
https://github.com/michaelrj-google created https://github.com/llvm/llvm-project/pull/98674
Reland of #97641 with sanitizer fixes
This adds the ability to erase a value from a blockstore based on an
iterator. For usability/testing purposes it also includes an addition
operator for blockstore's iterator.
>From 3f28437a58bba47ec0c53e9b0c801bf92f7bb343 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Tue, 2 Jul 2024 16:50:58 -0700
Subject: [PATCH 1/3] [libc] Add erase function to blockstore
This adds the ability to erase a value from a blockstore based on an
iterator. For usability/testing purposes it also includes an addition
operator for blockstore's iterator.
---
libc/src/__support/blockstore.h | 55 +++++++++++-
libc/test/src/__support/blockstore_test.cpp | 98 +++++++++++++++++++++
2 files changed, 151 insertions(+), 2 deletions(-)
diff --git a/libc/src/__support/blockstore.h b/libc/src/__support/blockstore.h
index 8d13e0ed290df..e9705f1fe27b4 100644
--- a/libc/src/__support/blockstore.h
+++ b/libc/src/__support/blockstore.h
@@ -9,9 +9,11 @@
#ifndef LLVM_LIBC_SRC___SUPPORT_BLOCKSTORE_H
#define LLVM_LIBC_SRC___SUPPORT_BLOCKSTORE_H
+#include "src/__support/CPP/array.h"
+#include "src/__support/CPP/new.h"
+#include "src/__support/CPP/type_traits.h"
+#include "src/__support/libc_assert.h"
#include "src/__support/macros/config.h"
-#include <src/__support/CPP/new.h>
-#include <src/__support/libc_assert.h>
#include <stddef.h>
#include <stdint.h>
@@ -98,6 +100,16 @@ class BlockStore {
return *reinterpret_cast<T *>(block->data + sizeof(T) * true_index);
}
+ LIBC_INLINE Iterator operator+(int i) {
+ LIBC_ASSERT(i >= 0 &&
+ "BlockStore iterators only support incrementation.");
+ auto other = *this;
+ for (int j = 0; j < i; ++j)
+ ++other;
+
+ return other;
+ }
+
LIBC_INLINE bool operator==(const Iterator &rhs) const {
return block == rhs.block && index == rhs.index;
}
@@ -176,6 +188,45 @@ class BlockStore {
else
return Iterator(current, fill_count);
}
+
+ // Removes and the element at pos, then moves all the objects after back by
+ // one to fill the hole. It's assumed that pos is a valid iterator to
+ // somewhere in this block_store.
+ LIBC_INLINE void erase(Iterator pos) {
+ const Iterator last_item = Iterator(current, fill_count);
+ if (pos == last_item) {
+ pop_back();
+ return;
+ }
+
+ if constexpr (REVERSE_ORDER) {
+ // REVERSE: Iterate from begin to pos
+ const Iterator range_end = pos;
+ Iterator cur = begin();
+ T prev_val = *cur;
+ ++cur;
+ T cur_val = *cur;
+
+ for (; cur != range_end; ++cur) {
+ cur_val = *cur;
+ *cur = prev_val;
+ prev_val = cur_val;
+ }
+ // We will always need to move at least one item (since we know that pos
+ // isn't the last item due to the check above).
+ *cur = prev_val;
+ } else {
+ // FORWARD: Iterate from pos to end
+ const Iterator range_end = end();
+ Iterator cur = pos;
+ Iterator prev = cur;
+ ++cur;
+
+ for (; cur != range_end; prev = cur, ++cur)
+ *prev = *cur;
+ }
+ pop_back();
+ }
};
template <typename T, size_t BLOCK_SIZE, bool REVERSE_ORDER>
diff --git a/libc/test/src/__support/blockstore_test.cpp b/libc/test/src/__support/blockstore_test.cpp
index 5fe8fef1b6edc..dd74ea18f2c02 100644
--- a/libc/test/src/__support/blockstore_test.cpp
+++ b/libc/test/src/__support/blockstore_test.cpp
@@ -64,6 +64,99 @@ class LlvmLibcBlockStoreTest : public LIBC_NAMESPACE::testing::Test {
}
block_store.destroy(&block_store);
}
+
+ template <bool REVERSE> void erase_test() {
+ using LIBC_NAMESPACE::BlockStore;
+ BlockStore<int, 2, REVERSE> block_store;
+ int i;
+
+ constexpr int ARR_SIZE = 6;
+
+ ASSERT_TRUE(block_store.empty());
+ for (int i = 0; i < ARR_SIZE; i++) {
+ ASSERT_TRUE(block_store.push_back(i + 1));
+ }
+
+ // block_store state should be {1,2,3,4,5,6}
+
+ block_store.erase(block_store.begin());
+
+ // FORWARD: block_store state should be {2,3,4,5,6}
+ // REVERSE: block_store state should be {1,2,3,4,5}
+
+ auto iter = block_store.begin();
+ for (i = 0; iter != block_store.end(); ++i, ++iter) {
+ if (!REVERSE) {
+ ASSERT_EQ(*iter, i + 2);
+ } else {
+ ASSERT_EQ(*iter, (ARR_SIZE - 1) - i);
+ }
+ }
+
+ // Assert that there were the correct number of elements
+ ASSERT_EQ(i, ARR_SIZE - 1);
+
+ block_store.erase(block_store.end());
+
+ // BOTH: block_store state should be {2,3,4,5}
+
+ iter = block_store.begin();
+ for (i = 0; iter != block_store.end(); ++i, ++iter) {
+ if (!REVERSE) {
+ ASSERT_EQ(*iter, i + 2);
+ } else {
+ ASSERT_EQ(*iter, (ARR_SIZE - 1) - i);
+ }
+ }
+
+ ASSERT_EQ(i, ARR_SIZE - 2);
+
+ block_store.erase(block_store.begin() + 1);
+
+ // FORWARD: block_store state should be {2,4,5}
+ // REVERSE: block_store state should be {2,3,5}
+
+ const int FORWARD_RESULTS[] = {2, 4, 5};
+ const int REVERSE_RESULTS[] = {2, 3, 5};
+
+ iter = block_store.begin();
+ for (i = 0; iter != block_store.end(); ++i, ++iter) {
+ if (!REVERSE) {
+ ASSERT_EQ(*iter, FORWARD_RESULTS[i]);
+ } else {
+ ASSERT_EQ(*iter, REVERSE_RESULTS[ARR_SIZE - 4 - i]); // reversed
+ }
+ }
+
+ ASSERT_EQ(i, ARR_SIZE - 3);
+
+ block_store.erase(block_store.begin() + 1);
+ // BOTH: block_store state should be {2,5}
+
+ iter = block_store.begin();
+ if (!REVERSE) {
+ ASSERT_EQ(*iter, 2);
+ ASSERT_EQ(*(iter + 1), 5);
+ } else {
+ ASSERT_EQ(*iter, 5);
+ ASSERT_EQ(*(iter + 1), 2);
+ }
+
+ block_store.erase(block_store.begin());
+ // FORWARD: block_store state should be {5}
+ // REVERSE: block_store state should be {2}
+ iter = block_store.begin();
+ if (!REVERSE) {
+ ASSERT_EQ(*iter, 5);
+ } else {
+ ASSERT_EQ(*iter, 2);
+ }
+
+ block_store.erase(block_store.begin());
+ // BOTH: block_store state should be {}
+
+ block_store.destroy(&block_store);
+ }
};
TEST_F(LlvmLibcBlockStoreTest, PopulateAndIterate4) {
@@ -100,3 +193,8 @@ TEST_F(LlvmLibcBlockStoreTest, Empty) {
empty_test<false>();
empty_test<true>();
}
+
+TEST_F(LlvmLibcBlockStoreTest, Erase) {
+ erase_test<false>();
+ erase_test<true>();
+}
>From 3c78a47191e952d070769493b394603e4d4e7c95 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Mon, 8 Jul 2024 14:01:18 -0700
Subject: [PATCH 2/3] fix typo
---
libc/src/__support/blockstore.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libc/src/__support/blockstore.h b/libc/src/__support/blockstore.h
index e9705f1fe27b4..41f5f196b7cef 100644
--- a/libc/src/__support/blockstore.h
+++ b/libc/src/__support/blockstore.h
@@ -189,9 +189,9 @@ class BlockStore {
return Iterator(current, fill_count);
}
- // Removes and the element at pos, then moves all the objects after back by
- // one to fill the hole. It's assumed that pos is a valid iterator to
- // somewhere in this block_store.
+ // Removes the element at pos, then moves all the objects after back by one to
+ // fill the hole. It's assumed that pos is a valid iterator to somewhere in
+ // this block_store.
LIBC_INLINE void erase(Iterator pos) {
const Iterator last_item = Iterator(current, fill_count);
if (pos == last_item) {
>From ef1c7f9348d2c56c6bd9f7aabfb6c6792d8e65eb Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Fri, 12 Jul 2024 11:15:04 -0700
Subject: [PATCH 3/3] fix sanitizer issue
---
libc/src/__support/blockstore.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/libc/src/__support/blockstore.h b/libc/src/__support/blockstore.h
index 41f5f196b7cef..efe2234eace59 100644
--- a/libc/src/__support/blockstore.h
+++ b/libc/src/__support/blockstore.h
@@ -212,9 +212,11 @@ class BlockStore {
*cur = prev_val;
prev_val = cur_val;
}
- // We will always need to move at least one item (since we know that pos
- // isn't the last item due to the check above).
- *cur = prev_val;
+ // As long as this isn't the end we will always need to move at least one
+ // item (since we know that pos isn't the last item due to the check
+ // above).
+ if (range_end != end())
+ *cur = prev_val;
} else {
// FORWARD: Iterate from pos to end
const Iterator range_end = end();
More information about the libc-commits
mailing list