Skip to content

Commit

Permalink
Fix resize() so that it respects max_size_max (#81)
Browse files Browse the repository at this point in the history 
* Fix resize() so that it respects max_size_max

#80

* Fix issues I found with push_back not throwing when beyond max_size_max

* Update from feedback and remove a redundant test
  • Loading branch information
@skeetsaz
skeetsaz authored Nov 13, 2023
1 parent e5181ac commit efc3904
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 48 deletions.
54 changes: 32 additions & 22 deletions cetlvast/suites/unittest/test_variable_length_array_bool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool>{true, false, true});
Expand Down Expand Up @@ -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
}
48 changes: 48 additions & 0 deletions cetlvast/suites/unittest/test_variable_length_array_general_allocation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,54 @@ TYPED_TEST(VLATestsGeneralAllocation, TestResizeWithCopy)

// +-------------------------------------------------------------------------------------------------------------------+

TYPED_TEST(VLATestsGeneralAllocation, TestExceedingMaxSizeMax)
{
if (!std::is_same<typename TypeParam::container_type, cetlvast::CETLTag>::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 =
Expand Down
47 changes: 21 additions & 26 deletions include/cetl/variable_length_array.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ class VariableLengthArrayBase
}

template <typename... Args>
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_)
{
Expand All @@ -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)
{
Expand Down Expand Up @@ -1403,12 +1409,7 @@ class VariableLengthArray : protected VariableLengthArrayBase<T, Allocator>
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);
}

///
Expand All @@ -1424,18 +1425,10 @@ class VariableLengthArray : protected VariableLengthArrayBase<T, Allocator>
{
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_type>(value));
}

///
Expand All @@ -1458,9 +1451,6 @@ class VariableLengthArray : protected VariableLengthArrayBase<T, Allocator>
{
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;
}

Expand Down Expand Up @@ -1522,19 +1512,24 @@ class VariableLengthArray : protected VariableLengthArrayBase<T, Allocator>
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_type>::value)
constexpr void push_back_impl(value_type&& value) noexcept(std::is_nothrow_move_constructible<value_type>::value)
{
CETL_DEBUG_ASSERT(size_ < capacity_, "CDE_vla_008: No capacity to push element.");
if (size_ < capacity_)
{
std::allocator_traits<allocator_type>::construct(alloc_, &data_[size_], std::move(value));
return &data_[size_++];
}
else
{
return nullptr;
size_++;
}
}

Expand Down

0 comments on commit efc3904

Please sign in to comment.