From 089cff9c0f71af00f1a76c66b32199778815b12e Mon Sep 17 00:00:00 2001 From: John Wellbelove Date: Sat, 25 Jul 2020 19:31:23 +0100 Subject: [PATCH] Fix pointer vector move operators Optimise constructors and assignments for pointer vectors --- include/etl/algorithm.h | 8 +- include/etl/private/ivectorpointer.h | 70 +++++-------- include/etl/private/pvoidvector.h | 21 +++- include/etl/vector.h | 104 ++++--------------- test/test_vector_pointer.cpp | 35 ++++++- test/test_vector_pointer_external_buffer.cpp | 27 +++++ 6 files changed, 133 insertions(+), 132 deletions(-) diff --git a/include/etl/algorithm.h b/include/etl/algorithm.h index d82038cb..0fb55434 100644 --- a/include/etl/algorithm.h +++ b/include/etl/algorithm.h @@ -191,8 +191,8 @@ namespace etl // Pointer template typename etl::enable_if::value && - etl::is_pointer::value && - etl::is_pod::value_type>::value, TIterator2>::type + etl::is_pointer::value && + etl::is_pod::value_type>::value, TIterator2>::type copy_n(TIterator1 sb, TSize count, TIterator2 db) { typedef typename etl::iterator_traits::value_type value_t; @@ -203,8 +203,8 @@ namespace etl // Other iterator template typename etl::enable_if::value || - !etl::is_pointer::value || - !etl::is_pod::value_type>::value, TIterator2>::type + !etl::is_pointer::value || + !etl::is_pod::value_type>::value, TIterator2>::type copy_n(TIterator1 sb, TSize count, TIterator2 db) { while (count != 0) diff --git a/include/etl/private/ivectorpointer.h b/include/etl/private/ivectorpointer.h index 22897ff0..46b9a07d 100644 --- a/include/etl/private/ivectorpointer.h +++ b/include/etl/private/ivectorpointer.h @@ -418,14 +418,23 @@ namespace etl //************************************************************************* ivector& operator = (const ivector& rhs) { - if (&rhs != this) - { - assign(rhs.cbegin(), rhs.cend()); - } + base_t::operator = (rhs); return *this; } +#if ETL_CPP11_SUPPORTED + //************************************************************************* + /// Move assignment operator. + //************************************************************************* + ivector& operator = (ivector&& rhs) + { + (void)base_t::operator = (etl::move(rhs)); + + return *this; + } +#endif + #ifdef ETL_IVECTOR_REPAIR_ENABLE //************************************************************************* /// Fix the internal pointers after a low level memory copy. @@ -442,24 +451,6 @@ namespace etl : pvoidvector(reinterpret_cast(p_buffer_), MAX_SIZE_) { } - - //********************************************************************* - /// Initialise the source vector after a move. - //********************************************************************* - void initialise_source_external_buffer_after_move() - { - ETL_SUBTRACT_DEBUG_COUNT(int32_t(etl::distance(p_buffer, p_end))) - - p_end = p_buffer; - } - - //********************************************************************* - /// Initialise the destination vector after a move. - //********************************************************************* - void initialise_destination_external_buffer_after_move() - { - ETL_ADD_DEBUG_COUNT(int32_t(etl::distance(p_buffer, p_end))) - } }; template @@ -836,14 +827,23 @@ namespace etl //************************************************************************* ivector& operator = (const ivector& rhs) { - if (&rhs != this) - { - assign(rhs.cbegin(), rhs.cend()); - } + base_t::operator = (rhs); return *this; } +#if ETL_CPP11_SUPPORTED + //************************************************************************* + /// Move assignment operator. + //************************************************************************* + ivector& operator = (ivector&& rhs) + { + (void)base_t::operator = (etl::move(rhs)); + + return *this; + } +#endif + #ifdef ETL_IVECTOR_REPAIR_ENABLE //************************************************************************* /// Fix the internal pointers after a low level memory copy. @@ -860,24 +860,6 @@ namespace etl : pvoidvector(reinterpret_cast(const_cast(p_buffer_)), MAX_SIZE_) { } - - //********************************************************************* - /// Initialise the source vector after a move. - //********************************************************************* - void initialise_source_external_buffer_after_move() - { - ETL_SUBTRACT_DEBUG_COUNT(int32_t(etl::distance(p_buffer, p_end))) - - p_end = p_buffer; - } - - //********************************************************************* - /// Initialise the destination vector after a move. - //********************************************************************* - void initialise_destination_external_buffer_after_move() - { - ETL_ADD_DEBUG_COUNT(int32_t(etl::distance(p_buffer, p_end))) - } }; //*************************************************************************** diff --git a/include/etl/private/pvoidvector.h b/include/etl/private/pvoidvector.h index 58963d8a..6801812c 100644 --- a/include/etl/private/pvoidvector.h +++ b/include/etl/private/pvoidvector.h @@ -489,12 +489,31 @@ namespace etl { if (&rhs != this) { - assign(rhs.cbegin(), rhs.cend()); + this->initialise(); + this->resize(rhs.size()); + etl::copy_n(rhs.data(), rhs.size(), this->data()); } return *this; } +#if ETL_CPP11_SUPPORTED + //************************************************************************* + /// Move assignment operator. + //************************************************************************* + etl::pvoidvector& operator = (etl::pvoidvector&& rhs) + { + if (&rhs != this) + { + this->initialise(); + this->resize(rhs.size()); + etl::copy_n(rhs.data(), rhs.size(), this->data()); + rhs.initialise(); + } + + return *this; + } +#endif //************************************************************************* /// Gets the current size of the vector. ///\return The current size of the vector. diff --git a/include/etl/vector.h b/include/etl/vector.h index 9b341655..f1ee479a 100644 --- a/include/etl/vector.h +++ b/include/etl/vector.h @@ -957,24 +957,6 @@ namespace etl p_end = p_buffer; } - //********************************************************************* - /// Initialise the source vector after a move. - //********************************************************************* - void initialise_source_external_buffer_after_move() - { - ETL_SUBTRACT_DEBUG_COUNT(int32_t(etl::distance(p_buffer, p_end))) - - p_end = p_buffer; - } - - //********************************************************************* - /// Initialise the destination vector after a move. - //********************************************************************* - void initialise_destination_external_buffer_after_move() - { - ETL_ADD_DEBUG_COUNT(int32_t(etl::distance(p_buffer, p_end))) - } - //************************************************************************* /// Fix the internal pointers after a low level memory copy. //************************************************************************* @@ -1404,11 +1386,14 @@ namespace etl { this->initialise(); - this->p_buffer = other.p_buffer; - this->p_end = other.p_end; + typename etl::ivector::iterator itr = other.begin(); + while (itr != other.end()) + { + this->push_back(etl::move(*itr)); + ++itr; + } - this->initialise_destination_external_buffer_after_move(); - other.initialise_source_external_buffer_after_move(); + other.initialise(); } } @@ -1420,11 +1405,15 @@ namespace etl if (&rhs != this) { this->clear(); - this->p_buffer = rhs.p_buffer; - this->p_end = rhs.p_end; - this->initialise_destination_external_buffer_after_move(); - rhs.initialise_source_external_buffer_after_move(); + typename etl::ivector::iterator itr = rhs.begin(); + while (itr != rhs.end()) + { + this->push_back(etl::move(*itr)); + ++itr; + } + + rhs.initialise(); } return *this; @@ -1530,10 +1519,7 @@ namespace etl vector(const vector& other) : etl::ivector(reinterpret_cast(&buffer), MAX_SIZE) { - if (this != &other) - { - this->assign(other.begin(), other.end()); - } + (void)etl::ivector::operator = (other); } //************************************************************************* @@ -1541,10 +1527,7 @@ namespace etl //************************************************************************* vector& operator = (const vector& rhs) { - if (&rhs != this) - { - this->assign(rhs.cbegin(), rhs.cend()); - } + (void)etl::ivector::operator = (rhs); return *this; } @@ -1556,19 +1539,7 @@ namespace etl vector(vector&& other) : etl::ivector(reinterpret_cast(&buffer), MAX_SIZE) { - if (this != &other) - { - this->initialise(); - - typename etl::ivector::iterator itr = other.begin(); - while (itr != other.end()) - { - this->push_back(etl::move(*itr)); - ++itr; - } - - other.initialise_source_external_buffer_after_move(); - } + (void)etl::ivector::operator = (etl::move(other)); } //************************************************************************* @@ -1576,18 +1547,7 @@ namespace etl //************************************************************************* vector& operator = (vector&& rhs) { - if (&rhs != this) - { - this->clear(); - typename etl::ivector::iterator itr = rhs.begin(); - while (itr != rhs.end()) - { - this->push_back(etl::move(*itr)); - ++itr; - } - - rhs.initialise_source_external_buffer_after_move(); - } + (void)etl::ivector::operator = (etl::move(rhs)); return *this; } @@ -1682,7 +1642,7 @@ namespace etl vector(const vector& other, void* buffer, size_t max_size) : etl::ivector(reinterpret_cast(buffer), max_size) { - this->assign(other.begin(), other.end()); + (void)etl::ivector::operator = (other); } //************************************************************************* @@ -1690,10 +1650,7 @@ namespace etl //************************************************************************* vector& operator = (const vector& rhs) { - if (&rhs != this) - { - this->assign(rhs.cbegin(), rhs.cend()); - } + (void)etl::ivector::operator = (rhs); return *this; } @@ -1705,14 +1662,7 @@ namespace etl vector(vector&& other, void* buffer, size_t max_size) : etl::ivector(reinterpret_cast(buffer), max_size) { - if (this != &other) - { - this->p_buffer = other.p_buffer; - this->p_end = other.p_end; - - this->initialise_destination_external_buffer_after_move(); - other.initialise_source_external_buffer_after_move(); - } + (void)etl::ivector::operator = (etl::move(other)); } //************************************************************************* @@ -1720,15 +1670,7 @@ namespace etl //************************************************************************* vector& operator = (vector&& rhs) { - if (&rhs != this) - { - this->clear(); - this->p_buffer = rhs.p_buffer; - this->p_end = rhs.p_end; - - this->initialise_destination_external_buffer_after_move(); - rhs.initialise_source_external_buffer_after_move(); - } + (void)etl::ivector::operator = (etl::move(rhs)); return *this; } diff --git a/test/test_vector_pointer.cpp b/test/test_vector_pointer.cpp index dd6896d5..849649de 100644 --- a/test/test_vector_pointer.cpp +++ b/test/test_vector_pointer.cpp @@ -281,6 +281,18 @@ namespace CHECK(data2 != data); } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_move_constructor) + { + Data data(initial_data.begin(), initial_data.end()); + Data data2(std::move(data)); + + CHECK_EQUAL(0U, data.size()); + CHECK_EQUAL(initial_data.size(), data2.size()); + + CHECK_EQUAL(initial_data.size(), data2.size()); + } + //************************************************************************* TEST_FIXTURE(SetupFixture, test_assignment) { @@ -289,8 +301,10 @@ namespace other_data = data; - bool is_equal = std::equal(data.begin(), data.end(), other_data.begin()); + CHECK_EQUAL(initial_data.size(), data.size()); + CHECK_EQUAL(initial_data.size(), other_data.size()); + bool is_equal = std::equal(data.begin(), data.end(), other_data.begin()); CHECK(is_equal); } @@ -302,8 +316,25 @@ namespace other_data = data; - bool is_equal = std::equal(data.begin(), data.end(), other_data.begin()); + CHECK_EQUAL(initial_data.size(), data.size()); + CHECK_EQUAL(initial_data.size(), other_data.size()); + bool is_equal = std::equal(data.begin(), data.end(), other_data.begin()); + CHECK(is_equal); + } + + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_move_assignment) + { + Data data(initial_data.begin(), initial_data.end()); + Data other_data; + + other_data = std::move(data); + + CHECK_EQUAL(0U, data.size()); + CHECK_EQUAL(initial_data.size(), other_data.size()); + + bool is_equal = std::equal(data.begin(), data.end(), other_data.begin()); CHECK(is_equal); } diff --git a/test/test_vector_pointer_external_buffer.cpp b/test/test_vector_pointer_external_buffer.cpp index 799cc643..fc6a39a4 100644 --- a/test/test_vector_pointer_external_buffer.cpp +++ b/test/test_vector_pointer_external_buffer.cpp @@ -290,6 +290,18 @@ namespace CHECK(data2 != data); } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_move_constructor) + { + Data data(initial_data.begin(), initial_data.end(), buffer1, SIZE); + Data data2(std::move(data), buffer2, SIZE); + + CHECK_EQUAL(0U, data.size()); + CHECK_EQUAL(initial_data.size(), data2.size()); + + CHECK_EQUAL(initial_data.size(), data2.size()); + } + //************************************************************************* TEST_FIXTURE(SetupFixture, test_assignment) { @@ -316,6 +328,21 @@ namespace CHECK(is_equal); } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_move_assignment) + { + Data data(initial_data.begin(), initial_data.end(), buffer1, SIZE); + Data other_data(buffer2, SIZE); + + other_data = std::move(data); + + CHECK_EQUAL(0U, data.size()); + CHECK_EQUAL(initial_data.size(), other_data.size()); + + bool is_equal = std::equal(data.begin(), data.end(), other_data.begin()); + CHECK(is_equal); + } + //************************************************************************* TEST_FIXTURE(SetupFixture, test_assignment_iterface) {