From efc3904c5a73550c32809051cf78e1b428271ee8 Mon Sep 17 00:00:00 2001 From: skeetsaz <120683575+skeetsaz@users.noreply.github.com> Date: Mon, 13 Nov 2023 09:50:43 -0800 Subject: [PATCH] Fix resize() so that it respects max_size_max (#81) * Fix resize() so that it respects max_size_max https://github.com/OpenCyphal/CETL/issues/80 * Fix issues I found with push_back not throwing when beyond max_size_max * Update from feedback and remove a redundant test --- .../test_variable_length_array_bool.cpp | 54 +++++++++++-------- ...riable_length_array_general_allocation.cpp | 48 +++++++++++++++++ include/cetl/variable_length_array.hpp | 47 ++++++++-------- 3 files changed, 101 insertions(+), 48 deletions(-) diff --git a/cetlvast/suites/unittest/test_variable_length_array_bool.cpp b/cetlvast/suites/unittest/test_variable_length_array_bool.cpp index 64a7fb79..c8c5e802 100644 --- a/cetlvast/suites/unittest/test_variable_length_array_bool.cpp +++ b/cetlvast/suites/unittest/test_variable_length_array_bool.cpp @@ -335,6 +335,38 @@ TYPED_TEST(VLABoolTests, TestBoolResizeOneBit) ASSERT_EQ(0, array[4]); } +TYPED_TEST(VLABoolTestsVLAOnly, TestBoolExceedingMaxSizeMax) +{ + std::size_t max_size_max = 1ul; + auto array = TypeParam::make_bool_container(max_size_max); + array.push_back(true); + ASSERT_EQ(1, array.size()); + + // Test resize() +#ifdef __cpp_exceptions + ASSERT_THROW(array.resize(2 * max_size_max), std::length_error); +#else + array.resize(2 * max_size_max); + ASSERT_EQ(max_size_max, array.size()); +#endif // __cpp_exceptions + + // Test push_back() +#ifdef __cpp_exceptions + ASSERT_THROW(array.push_back(true), std::length_error); +#else + array.push_back(true); + ASSERT_EQ(max_size_max, array.size()); +#endif // __cpp_exceptions + + // Test emplace_back() +#ifdef __cpp_exceptions + ASSERT_THROW(array.emplace_back(true), std::length_error); +#else + array.emplace_back(true); + ASSERT_EQ(max_size_max, array.size()); +#endif // __cpp_exceptions +} + TYPED_TEST(VLABoolTests, TestBoolFront) { auto array = TypeParam::make_bool_container(std::initializer_list{true, false, true}); @@ -396,25 +428,3 @@ TYPED_TEST(VLABoolTestsVLAOnly, ConstructFromIteratorRange) GTEST_SKIP() << "C++17 pmr does not support defined out of memory behaviour without exceptions."; #endif } - -TYPED_TEST(VLABoolTestsVLAOnly, ExceedMaxSizeMaxFails) -{ - std::size_t max = 3U; - auto subject = TypeParam::make_bool_container(max); - for (std::size_t i = 0; i < max; i++) { - subject.push_back(true); - } -#if defined(__cpp_exceptions) - EXPECT_THROW((void)subject.push_back(true), std::length_error); -#elif (__cplusplus == CETL_CPP_STANDARD_14) - // Try to add one too many - subject.push_back(true); - // Shouldn't have been added - EXPECT_EQ(subject.size(), 3); - EXPECT_EQ(subject[0], true); - EXPECT_EQ(subject[1], true); - EXPECT_EQ(subject[2], true); -#else - GTEST_SKIP() << "C++17 pmr does not support defined out of memory behaviour without exceptions."; -#endif -} diff --git a/cetlvast/suites/unittest/test_variable_length_array_general_allocation.cpp b/cetlvast/suites/unittest/test_variable_length_array_general_allocation.cpp index aa928916..4a8599cb 100644 --- a/cetlvast/suites/unittest/test_variable_length_array_general_allocation.cpp +++ b/cetlvast/suites/unittest/test_variable_length_array_general_allocation.cpp @@ -784,6 +784,54 @@ TYPED_TEST(VLATestsGeneralAllocation, TestResizeWithCopy) // +-------------------------------------------------------------------------------------------------------------------+ +TYPED_TEST(VLATestsGeneralAllocation, TestExceedingMaxSizeMax) +{ + if (!std::is_same::value) + { + GTEST_SKIP() << "Skipping test that only works for CETL VLA."; + } + + std::size_t max_size_max = 1ul; + typename TestFixture::SubjectType subject{max_size_max, TestFixture::make_allocator()}; + subject.push_back(123ul); + ASSERT_EQ(1, subject.size()); + + // Test resize() +#ifdef __cpp_exceptions + ASSERT_THROW(subject.resize(2 * max_size_max), std::length_error); +#else + subject.resize(2 * max_size_max); + ASSERT_EQ(max_size_max, subject.size()); +#endif // __cpp_exceptions + + // Test push_back() +#ifdef __cpp_exceptions + const auto num = 456ul; // have to pass a const arg to hit the right push_back + ASSERT_THROW(subject.push_back(num), std::length_error); +#else + subject.push_back(456ul); + ASSERT_EQ(max_size_max, subject.size()); +#endif // __cpp_exceptions + + // Test move push_back() +#ifdef __cpp_exceptions + ASSERT_THROW(subject.push_back(std::move(456ul)), std::length_error); +#else + subject.push_back(std::move(456ul)); + ASSERT_EQ(max_size_max, subject.size()); +#endif // __cpp_exceptions + + // Test emplace_back() +#ifdef __cpp_exceptions + ASSERT_THROW(subject.emplace_back(456ul), std::length_error); +#else + subject.emplace_back(456ul); + ASSERT_EQ(max_size_max, subject.size()); +#endif // __cpp_exceptions +} + +// +-------------------------------------------------------------------------------------------------------------------+ + TYPED_TEST(VLATestsGeneralAllocation, TestFrontAndBack) { using const_ref_value_type = diff --git a/include/cetl/variable_length_array.hpp b/include/cetl/variable_length_array.hpp index fc0ba395..5f8f9172 100644 --- a/include/cetl/variable_length_array.hpp +++ b/include/cetl/variable_length_array.hpp @@ -646,7 +646,7 @@ class VariableLengthArrayBase } template - constexpr void resize(const size_type new_size, const size_type max_size, Args&&... args) + constexpr void resize(size_type new_size, const size_type max_size, Args&&... args) { if (new_size == size_) { @@ -659,6 +659,12 @@ class VariableLengthArrayBase if (new_size > capacity_) { reserve(new_size, max_size); +#if !defined(__cpp_exceptions) + if (capacity_ != new_size) + { + new_size = capacity_; + } +#endif } for (std::size_t i = size_; i < new_size; ++i) { @@ -1403,12 +1409,7 @@ class VariableLengthArray : protected VariableLengthArrayBase return; } - if (nullptr == push_back_impl(value)) - { -#if defined(__cpp_exceptions) - throw std::length_error("size is at capacity. Use reserve to grow the capacity."); -#endif - } + push_back_impl(value); } /// @@ -1424,18 +1425,10 @@ class VariableLengthArray : protected VariableLengthArrayBase { if (!ensure_size_plus_one()) { -#if defined(__cpp_exceptions) - throw std::length_error("size is at capacity and we cannot grow the capacity."); -#endif return; } - if (nullptr == push_back_impl(std::move(value))) - { -#if defined(__cpp_exceptions) - throw std::length_error("size is at capacity. Use reserve to grow the capacity."); -#endif - } + push_back_impl(std::forward(value)); } /// @@ -1458,9 +1451,6 @@ class VariableLengthArray : protected VariableLengthArrayBase { if (!ensure_size_plus_one()) { -#if defined(__cpp_exceptions) - throw std::length_error("size is at capacity and we cannot grow the capacity."); -#endif return; } @@ -1522,19 +1512,24 @@ class VariableLengthArray : protected VariableLengthArrayBase return true; } - return Base::grow(max_size()); + if (!Base::grow(max_size())) + { +#if defined(__cpp_exceptions) + throw std::length_error("size is at capacity and we cannot grow the capacity."); +#endif + return false; + } + + return true; } - constexpr pointer push_back_impl(value_type&& value) noexcept(std::is_nothrow_move_constructible::value) + constexpr void push_back_impl(value_type&& value) noexcept(std::is_nothrow_move_constructible::value) { + CETL_DEBUG_ASSERT(size_ < capacity_, "CDE_vla_008: No capacity to push element."); if (size_ < capacity_) { std::allocator_traits::construct(alloc_, &data_[size_], std::move(value)); - return &data_[size_++]; - } - else - { - return nullptr; + size_++; } }