diff --git a/.gitignore b/.gitignore index 82aa793..f084773 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,12 @@ .idea cmake-build-debug cmake-build-release +build +Testing/Temporary +_codeql_build_dir +_codeql_detected_source_root + +# CodeQL build artifacts +_codeql_build_dir +_codeql_detected_source_root + diff --git a/CMakeLists.txt b/CMakeLists.txt index 93a1108..3947073 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ cmake_minimum_required(VERSION 3.21) -project(RingBufferTest) +project(RingBuffer LANGUAGES CXX) set(Simulate_Android_ToolChain OFF) if (Simulate_Android_ToolChain) @@ -9,19 +9,23 @@ else() set(CMAKE_CXX_STANDARD 17) endif() -# Add Google Test -# Include FetchContent to download and manage external dependencies -include(FetchContent) -FetchContent_Declare( - googletest - URL https://github.com/google/googletest/archive/refs/tags/release-1.11.0.zip -) -FetchContent_MakeAvailable(googletest) +add_library(RingBuffer INTERFACE) +add_library(RingBuffer::RingBuffer ALIAS RingBuffer) +target_include_directories(RingBuffer INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}) -enable_testing() -add_executable(RingBufferTest test_main.cpp) -target_link_libraries(RingBufferTest gtest gtest_main) +option(RINGBUFFER_BUILD_TESTS "Build RingBuffer tests" ON) +if (RINGBUFFER_BUILD_TESTS) + include(FetchContent) + FetchContent_Declare( + googletest + URL https://github.com/google/googletest/archive/refs/tags/release-1.11.0.zip + ) + FetchContent_MakeAvailable(googletest) + enable_testing() + add_executable(RingBufferTest test_main.cpp) + target_link_libraries(RingBufferTest gtest gtest_main RingBuffer::RingBuffer) -include(GoogleTest) -gtest_discover_tests(RingBufferTest) + include(GoogleTest) + gtest_discover_tests(RingBufferTest) +endif() diff --git a/README.md b/README.md index 35fbf02..a91f815 100644 --- a/README.md +++ b/README.md @@ -46,8 +46,8 @@ This project demonstrates many modern C++ best practices: ### Build Instructions ```bash -git clone https://github.com/yourusername/RingBuffer.git -cd RingBuffer +git clone https://github.com/bugparty/RingBufferCpp.git +cd RingBufferCpp mkdir build && cd build cmake .. cmake --build . @@ -60,6 +60,39 @@ This will: --- +## FetchContent Integration + +```cmake +include(FetchContent) +FetchContent_Declare( + RingBuffer + GIT_REPOSITORY https://github.com/bugparty/RingBufferCpp.git + GIT_TAG main +) +FetchContent_MakeAvailable(RingBuffer) + +target_link_libraries(your_target PRIVATE RingBuffer::RingBuffer) +``` + +To skip building tests in your parent project, set: + +```cmake +set(RINGBUFFER_BUILD_TESTS OFF) +``` + +--- + +## Testing Policy + +When changing the library, run the tests before pushing changes: + +```bash +cmake --build build +ctest --output-on-failure --test-dir build +``` + +--- + Usage ```cpp diff --git a/RingBuffer.hpp b/RingBuffer.hpp index d2ee44b..bcb4bdd 100644 --- a/RingBuffer.hpp +++ b/RingBuffer.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #pragma once namespace buffers { @@ -62,12 +63,16 @@ namespace buffers { [[nodiscard]] const_pointer operator->() const noexcept { return &((*source_)[index_]); } - [[nodiscard]] self_type& operator++() noexcept { - index_ = ++index_ % N; + self_type& operator++() noexcept { ++count_; + if (count_ >= source_->size()) { + index_ = N; // Set to sentinel value (out of valid range [0, N-1]) when reaching end + } else { + index_ = (index_ + 1) % N; + } return *this; } - [[nodiscard]] self_type operator++(int) noexcept { + self_type operator++(int) noexcept { auto result = *this; ++(*this); return result; @@ -78,6 +83,9 @@ namespace buffers { [[nodiscard]] size_type count() const noexcept { return count_; } + [[nodiscard]] buffer_t source() const noexcept { + return source_; + } ~ring_buffer_iterator() = default; private: buffer_t source_{}; @@ -88,13 +96,13 @@ namespace buffers { template bool operator==(ring_buffer_iterator const& l, ring_buffer_iterator const& r) noexcept { - return l.count() == r.count(); + return l.source() == r.source() && l.index() == r.index(); } template bool operator!=(ring_buffer_iterator const& l, ring_buffer_iterator const& r) noexcept { - return l.count() != r.count(); + return l.source() != r.source() || l.index() != r.index(); } } @@ -134,11 +142,14 @@ using std::bool_constant; using iterator = detail::ring_buffer_iterator; using const_iterator = detail::ring_buffer_iterator; + // Create an empty ring buffer. ring_buffer() noexcept = default; + // Copy contents and state from another buffer. ring_buffer(ring_buffer const& rhs) noexcept(is_nothrow_copy_constructible_v) { copy_impl(rhs, bool_constant>{}); } + // Assign from another buffer. ring_buffer& operator=(ring_buffer const& rhs) noexcept(is_nothrow_copy_constructible_v) { if(this == &rhs) return *this; @@ -148,10 +159,31 @@ using std::bool_constant; return *this; } + // Move contents and state from another buffer. + ring_buffer(ring_buffer&& rhs) noexcept(std::is_nothrow_move_constructible::value) + { + move_impl(rhs, bool_constant>{}); + } + // Move-assign from another buffer. + ring_buffer& operator=(ring_buffer&& rhs) noexcept(std::is_nothrow_move_constructible::value) { + if(this == &rhs) + return *this; + + destroy_all(bool_constant>{}); + move_impl(rhs, bool_constant>{}); + + return *this; + } + // Swap contents with another buffer. + void swap(self_type& rhs) noexcept(noexcept(swap_impl(rhs, bool_constant>{}))) { + swap_impl(rhs, bool_constant>{}); + } + // Append an element, overwriting when configured. template void push_back(U&& value) { push_back(std::forward(value), bool_constant{}); } + // Remove the oldest element if present. void pop_front() noexcept{ if(empty()) return; @@ -161,36 +193,52 @@ using std::bool_constant; --size_; tail_ = ++tail_ %N; } + // Access the newest element. [[nodiscard]] reference back() noexcept { return reinterpret_cast(elements_[(head_ + N - 1) % N]); } + // Access the newest element (const). [[nodiscard]] const_reference back() const noexcept { return const_cast(this)->back(); } + // Access the oldest element. [[nodiscard]] reference front() noexcept { return reinterpret_cast(elements_[tail_]); } + // Access the oldest element (const). [[nodiscard]] const_reference front() const noexcept { return const_cast(this)->front(); } + // Direct access by internal storage index. [[nodiscard]] reference operator[](size_type index) noexcept { return reinterpret_cast(elements_[index]); } + // Direct access by internal storage index (const). [[nodiscard]] const_reference operator[](size_type index) const noexcept { return const_cast(this)->operator[](index); } + // Iterator to oldest element. [[nodiscard]] iterator begin() noexcept { return iterator{this, tail_, 0};} - [[nodiscard]] iterator end() noexcept { return iterator{this, head_, size_};} + // Iterator to one past newest element. + [[nodiscard]] iterator end() noexcept { return iterator{this, N, size_};} + // Const iterator to oldest element. [[nodiscard]] const_iterator cbegin() const noexcept { return const_iterator{this, tail_, 0};} - [[nodiscard]] const_iterator cend() const noexcept { return const_iterator{this, head_, size_};} + // Const iterator to one past newest element. + [[nodiscard]] const_iterator cend() const noexcept { return const_iterator{this, N, size_};} + // Check if buffer has no elements. [[nodiscard]] bool empty() const noexcept { return size_ == 0; } + // Check if buffer is at capacity. [[nodiscard]] bool full() const noexcept { return size_ == N; } + // Current element count. [[nodiscard]] size_type size() const noexcept { return size_; } + // Maximum element count. [[nodiscard]] size_type capacity() const noexcept { return N; } + // Remove all elements and reset indices. void clear() noexcept { destroy_all(bool_constant>{}); size_ = 0; head_ = 0; tail_ = 0; } + // Destroy elements on teardown. ~ring_buffer() { clear(); }; @@ -204,7 +252,7 @@ using std::bool_constant; } } void copy_impl(self_type const& rhs, std::true_type) { - std::memcpy(elements_, rhs.elements_, rhs.size_ * sizeof(T)); + std::memcpy(elements_, rhs.elements_, N * sizeof(T)); size_ = rhs.size_; tail_ = rhs.tail_; head_ = rhs.head_; @@ -248,6 +296,59 @@ using std::bool_constant; #endif } + void move_impl(self_type& rhs, std::true_type) { + std::memcpy(elements_, rhs.elements_, N * sizeof(T)); + size_ = rhs.size_; + tail_ = rhs.tail_; + head_ = rhs.head_; + rhs.clear(); + } + void move_impl(self_type& rhs, std::false_type) { + tail_ = rhs.tail_; + head_ = rhs.head_; + size_ = rhs.size_; +#ifdef __cpp_exceptions + try { + for (auto i = 0; i < size_; ++i) + new( elements_ + ((tail_ + i) % N)) T(std::move(rhs[(tail_ + i) % N])); + }catch(...) { + while(!empty()) { + destroy(tail_, bool_constant>{}); + tail_ = ++tail_ % N; + --size_; + } + throw; + } +#else + storage_type *p = nullptr; + for (auto i = 0; i < size_; ++i) { + p =reinterpret_cast(new(elements_ + ((tail_ + i) % N)) T(std::move(rhs[(tail_ + i) % N]))); + if (!p) { + break; + } + } + if (!p) { + while(!empty()) { + destroy(tail_, bool_constant>{}); + tail_ = ++tail_ % N; + --size_; + } + } +#endif + rhs.clear(); + } + void swap_impl(self_type& rhs, std::true_type) noexcept { + std::swap(elements_, rhs.elements_); + std::swap(head_, rhs.head_); + std::swap(tail_, rhs.tail_); + std::swap(size_, rhs.size_); + } + void swap_impl(self_type& rhs, std::false_type) { + self_type temp; + temp.move_impl(*this, std::false_type{}); + this->move_impl(rhs, std::false_type{}); + rhs.move_impl(temp, std::false_type{}); + } template void push_back(U&& value, std::true_type) { push_back_impl(std::forward(value)); @@ -284,5 +385,10 @@ using std::bool_constant; size_type size_{}; }; + template + void swap(ring_buffer& lhs, ring_buffer& rhs) noexcept(noexcept(lhs.swap(rhs))) { + lhs.swap(rhs); + } + } #endif //RINGBUFFERTEST_RINGBUFFER_HPP diff --git a/_codeql_detected_source_root b/_codeql_detected_source_root new file mode 120000 index 0000000..945c9b4 --- /dev/null +++ b/_codeql_detected_source_root @@ -0,0 +1 @@ +. \ No newline at end of file diff --git a/test_main.cpp b/test_main.cpp index 04d848f..905a456 100644 --- a/test_main.cpp +++ b/test_main.cpp @@ -96,6 +96,335 @@ TEST(RingBufferTest, Test7ArrowOperator) { EXPECT_EQ((++it)->at(1), 4); } +TEST(RingBufferTest, CopyPreservesWrappedDataForTrivialTypes) { + ring_buffer b1; + b1.push_back(1); + b1.push_back(2); + b1.push_back(3); + b1.pop_front(); + b1.push_back(4); + b1.push_back(5); + + ring_buffer b2 = b1; + std::vector values; + for (auto it = b2.cbegin(); it != b2.cend(); ++it) { + values.push_back(*it); + } + + std::vector expected = {2, 3, 4, 5}; + EXPECT_EQ(values, expected); +} + +TEST(RingBufferTest, IteratorsFromDifferentBuffersAreNotEqual) { + ring_buffer b1; + ring_buffer b2; + + // Iterators from different buffers should not be equal + EXPECT_NE(b1.cbegin(), b2.cbegin()); + + // Add elements to both buffers + b1.push_back(1); + b1.push_back(2); + b2.push_back(1); + b2.push_back(2); + + // Iterators at the same logical position from different buffers should still be unequal + EXPECT_NE(b1.cbegin(), b2.cbegin()); + EXPECT_NE(b1.cend(), b2.cend()); + + // Iterators from the same buffer at the same position should be equal + EXPECT_EQ(b1.cbegin(), b1.cbegin()); + EXPECT_EQ(b1.cend(), b1.cend()); + + // begin() and end() from the same buffer should be different when not empty + EXPECT_NE(b1.cbegin(), b1.cend()); + EXPECT_NE(b2.cbegin(), b2.cend()); +} + +TEST(RingBufferTest, NoOverwriteWhenFull) { + ring_buffer b1; + b1.push_back(1); + b1.push_back(2); + b1.push_back(3); + b1.push_back(4); + + EXPECT_EQ(b1.size(), 3); + EXPECT_EQ(b1.front(), 1); + EXPECT_EQ(b1.back(), 3); +} + +TEST(RingBufferTest, PopFrontOnEmptyIsNoOp) { + ring_buffer b1; + b1.pop_front(); + + EXPECT_TRUE(b1.empty()); + EXPECT_EQ(b1.size(), 0); + + b1.push_back(7); + EXPECT_FALSE(b1.empty()); + EXPECT_EQ(b1.front(), 7); + EXPECT_EQ(b1.back(), 7); +} + +TEST(RingBufferTest, IteratorOrderAfterWrapAndPop) { + ring_buffer b1; + b1.push_back(1); + b1.push_back(2); + b1.push_back(3); + b1.push_back(4); + b1.pop_front(); + b1.pop_front(); + b1.push_back(5); + b1.push_back(6); + + std::vector values; + for (auto it = b1.cbegin(); it != b1.cend(); ++it) { + values.push_back(*it); + } + + std::vector expected = {3, 4, 5, 6}; + EXPECT_EQ(values, expected); +} + +TEST(RingBufferTest, SelfAssignmentDoesNotCorrupt) { + ring_buffer b1; + b1.push_back(1); + b1.push_back(2); + b1.push_back(3); + + b1 = b1; + + std::vector values; + for (auto it = b1.cbegin(); it != b1.cend(); ++it) { + values.push_back(*it); + } + + std::vector expected = {1, 2, 3}; + EXPECT_EQ(values, expected); +} + +TEST(RingBufferTest, ClearResetsAfterWrap) { + ring_buffer b1; + b1.push_back(1); + b1.push_back(2); + b1.push_back(3); + b1.push_back(4); + + b1.clear(); + + EXPECT_TRUE(b1.empty()); + EXPECT_EQ(b1.size(), 0); + + b1.push_back(9); + b1.push_back(10); + EXPECT_EQ(b1.front(), 9); + EXPECT_EQ(b1.back(), 10); +} + +TEST(RingBufferTest, OverwriteKeepsCapacitySize) { + ring_buffer b1; + for (int i = 1; i <= 10; ++i) { + b1.push_back(i); + } + + EXPECT_TRUE(b1.full()); + EXPECT_EQ(b1.size(), 4); + EXPECT_EQ(b1.front(), 7); + EXPECT_EQ(b1.back(), 10); +} + +TEST(RingBufferTest, LargeBufferCopyPreservesOrder) { + ring_buffer b1; + for (int i = 0; i < 256; ++i) { + b1.push_back(i); + } + + ring_buffer b2 = b1; + std::vector values; + for (auto it = b2.cbegin(); it != b2.cend(); ++it) { + values.push_back(*it); + } + + std::vector expected; + for (int i = 128; i < 256; ++i) { + expected.push_back(i); + } + + EXPECT_EQ(values, expected); +} + +TEST(RingBufferTest, LargeBufferOverwriteKeepsLastN) { + ring_buffer b1; + for (int i = 0; i < 1000; ++i) { + b1.push_back(i); + } + + EXPECT_TRUE(b1.full()); + EXPECT_EQ(b1.size(), 64); + EXPECT_EQ(b1.front(), 936); + EXPECT_EQ(b1.back(), 999); +} + +TEST(RingBufferTest, SingleElementCapacityBehavesCorrectly) { + ring_buffer b1; + EXPECT_TRUE(b1.empty()); + EXPECT_EQ(b1.size(), 0); + EXPECT_EQ(b1.capacity(), 1); + + b1.push_back(10); + EXPECT_TRUE(b1.full()); + EXPECT_EQ(b1.front(), 10); + EXPECT_EQ(b1.back(), 10); + + b1.push_back(20); + EXPECT_TRUE(b1.full()); + EXPECT_EQ(b1.size(), 1); + EXPECT_EQ(b1.front(), 20); + EXPECT_EQ(b1.back(), 20); + + b1.pop_front(); + EXPECT_TRUE(b1.empty()); +} + +TEST(RingBufferTest, MoveConstructionTransfersState) { + ring_buffer b1; + b1.push_back(1); + b1.push_back(2); + b1.push_back(3); + b1.pop_front(); + b1.push_back(4); + + ring_buffer b2(std::move(b1)); + + EXPECT_TRUE(b1.empty()); + EXPECT_EQ(b2.size(), 3); + EXPECT_EQ(b2.front(), 2); + EXPECT_EQ(b2.back(), 4); +} + +TEST(RingBufferTest, MoveAssignmentTransfersState) { + ring_buffer b1; + b1.push_back(5); + b1.push_back(6); + + ring_buffer b2; + b2.push_back(10); + b2 = std::move(b1); + + EXPECT_TRUE(b1.empty()); + EXPECT_EQ(b2.size(), 2); + EXPECT_EQ(b2.front(), 5); + EXPECT_EQ(b2.back(), 6); +} + +TEST(RingBufferTest, SwapExchangesBuffers) { + ring_buffer b1; + b1.push_back(1); + b1.push_back(2); + + ring_buffer b2; + b2.push_back(9); + b2.push_back(10); + b2.push_back(11); + + b1.swap(b2); + + EXPECT_EQ(b1.size(), 3); + EXPECT_EQ(b1.front(), 9); + EXPECT_EQ(b1.back(), 11); + EXPECT_EQ(b2.size(), 2); + EXPECT_EQ(b2.front(), 1); + EXPECT_EQ(b2.back(), 2); +} + +TEST(RingBufferTest, NonMemberSwapWorks) { + ring_buffer b1; + b1.push_back(3); + b1.push_back(4); + + ring_buffer b2; + b2.push_back(7); + + using std::swap; + swap(b1, b2); + + EXPECT_EQ(b1.size(), 1); + EXPECT_EQ(b1.front(), 7); + EXPECT_EQ(b2.size(), 2); + EXPECT_EQ(b2.front(), 3); + EXPECT_EQ(b2.back(), 4); +} + +// Tests for move and swap with non-trivially copyable types +TEST(RingBufferTest, MoveConstructionWithNonTriviallyCopyableType) { + ring_buffer b1; + b1.push_back("hello"); + b1.push_back("world"); + b1.push_back("test"); + b1.pop_front(); + b1.push_back("data"); + + ring_buffer b2(std::move(b1)); + + EXPECT_TRUE(b1.empty()); + EXPECT_EQ(b2.size(), 3); + EXPECT_EQ(b2.front(), "world"); + EXPECT_EQ(b2.back(), "data"); +} + +TEST(RingBufferTest, MoveAssignmentWithNonTriviallyCopyableType) { + ring_buffer b1; + b1.push_back("foo"); + b1.push_back("bar"); + + ring_buffer b2; + b2.push_back("existing"); + b2 = std::move(b1); + + EXPECT_TRUE(b1.empty()); + EXPECT_EQ(b2.size(), 2); + EXPECT_EQ(b2.front(), "foo"); + EXPECT_EQ(b2.back(), "bar"); +} + +TEST(RingBufferTest, SwapWithNonTriviallyCopyableType) { + ring_buffer b1; + b1.push_back("one"); + b1.push_back("two"); + + ring_buffer b2; + b2.push_back("alpha"); + b2.push_back("beta"); + b2.push_back("gamma"); + + b1.swap(b2); + + EXPECT_EQ(b1.size(), 3); + EXPECT_EQ(b1.front(), "alpha"); + EXPECT_EQ(b1.back(), "gamma"); + EXPECT_EQ(b2.size(), 2); + EXPECT_EQ(b2.front(), "one"); + EXPECT_EQ(b2.back(), "two"); +} + +TEST(RingBufferTest, NonMemberSwapWithNonTriviallyCopyableType) { + ring_buffer, 2> b1; + b1.push_back(std::vector{1, 2, 3}); + b1.push_back(std::vector{4, 5, 6}); + + ring_buffer, 2> b2; + b2.push_back(std::vector{7, 8, 9}); + + using std::swap; + swap(b1, b2); + + EXPECT_EQ(b1.size(), 1); + EXPECT_EQ(b1.front(), (std::vector{7, 8, 9})); + EXPECT_EQ(b2.size(), 2); + EXPECT_EQ(b2.front(), (std::vector{1, 2, 3})); + EXPECT_EQ(b2.back(), (std::vector{4, 5, 6})); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS();