[libc-commits] [libc] Revert "[libc] Add erase function to blockstore" (PR #98669)
Michael Jones via libc-commits
libc-commits at lists.llvm.org
Fri Jul 12 10:51:24 PDT 2024
https://github.com/michaelrj-google created https://github.com/llvm/llvm-project/pull/98669
Reverts llvm/llvm-project#97641
Fails under sanitizers
>From 6ea50a98e3f6db5c0fbd3ff6c16e2d9ac01fae37 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Fri, 12 Jul 2024 10:50:56 -0700
Subject: [PATCH] Revert "[libc] Add erase function to blockstore (#97641)"
This reverts commit 9e452c1af430e5090de92aebfe422abbf5e51ff3.
---
libc/src/__support/blockstore.h | 55 +-----------
libc/test/src/__support/blockstore_test.cpp | 98 ---------------------
2 files changed, 2 insertions(+), 151 deletions(-)
diff --git a/libc/src/__support/blockstore.h b/libc/src/__support/blockstore.h
index 41f5f196b7cef..8d13e0ed290df 100644
--- a/libc/src/__support/blockstore.h
+++ b/libc/src/__support/blockstore.h
@@ -9,11 +9,9 @@
#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>
@@ -100,16 +98,6 @@ 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;
}
@@ -188,45 +176,6 @@ class BlockStore {
else
return Iterator(current, fill_count);
}
-
- // 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) {
- 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 dd74ea18f2c02..5fe8fef1b6edc 100644
--- a/libc/test/src/__support/blockstore_test.cpp
+++ b/libc/test/src/__support/blockstore_test.cpp
@@ -64,99 +64,6 @@ 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) {
@@ -193,8 +100,3 @@ TEST_F(LlvmLibcBlockStoreTest, Empty) {
empty_test<false>();
empty_test<true>();
}
-
-TEST_F(LlvmLibcBlockStoreTest, Erase) {
- erase_test<false>();
- erase_test<true>();
-}
More information about the libc-commits
mailing list