From 40fc6174666c79a92007bdf0d04747b48bafd2c3 Mon Sep 17 00:00:00 2001 From: John Wellbelove Date: Thu, 30 Jul 2020 12:52:40 +0100 Subject: [PATCH] Fix and optimise etl::list and etl::forward_list shared pool move constructors and assignment operators. --- include/etl/forward_list.h | 6 +- include/etl/list.h | 128 ++++++++++++++++++++++----------- include/etl/version.h | 2 +- library.json | 2 +- library.properties | 2 +- support/Release notes.txt | 4 ++ test/test_list_shared_pool.cpp | 112 ++++++++++++++++++++++++++++- 7 files changed, 207 insertions(+), 49 deletions(-) diff --git a/include/etl/forward_list.h b/include/etl/forward_list.h index 5a58e35b..3db42f3f 100644 --- a/include/etl/forward_list.h +++ b/include/etl/forward_list.h @@ -1492,8 +1492,8 @@ namespace etl node_t* p_node = p_rhs_node; p_rhs_node = p_rhs_node->next; - join(p_last_node, p_node); - p_node->next = ETL_NULLPTR; + insert_node_after(*p_last_node, *p_node); + p_last_node = p_node; ETL_INCREMENT_DEBUG_COUNT; @@ -1899,7 +1899,7 @@ namespace etl //************************************************************************* etl::ipool& get_pool() const { - return *p_node_pool; + return *this->p_node_pool; } }; diff --git a/include/etl/list.h b/include/etl/list.h index 47e422e1..be4c1856 100644 --- a/include/etl/list.h +++ b/include/etl/list.h @@ -1829,6 +1829,59 @@ namespace etl join(terminal_node, terminal_node); } +#if ETL_CPP11_SUPPORTED + //************************************************************************* + /// Move a forward list + //************************************************************************* + void move_container(ilist&& rhs) + { + if (&rhs != this) + { + this->initialise(); + + if (!rhs.empty()) + { + // Are we using the same pool? + if (this->get_node_pool() == rhs.get_node_pool()) + { + node_t* p_rhs_node = &rhs.get_head(); + + // Just link the nodes to the new forward_list. + do + { + ETL_ASSERT(!full(), ETL_ERROR(list_full)); + + node_t* p_node = p_rhs_node; + p_rhs_node = p_rhs_node->next; + insert_node(terminal_node, *p_node); + + ETL_INCREMENT_DEBUG_COUNT; + + } while (p_rhs_node != &rhs.terminal_node); + + rhs.ETL_RESET_DEBUG_COUNT; + rhs.join(rhs.terminal_node, rhs.terminal_node); + } + else + { + // Add all of the elements. + etl::ilist::iterator first = rhs.begin(); + etl::ilist::iterator last = rhs.end(); + + while (first != last) + { + ETL_ASSERT(!full(), ETL_ERROR(list_full)); + + insert_node(terminal_node, this->allocate_data_node(etl::move(*first++))); + } + + rhs.initialise(); + } + } + } + } +#endif + private: //************************************************************************* @@ -2090,23 +2143,11 @@ namespace etl #if ETL_CPP11_SUPPORTED //************************************************************************* - /// Assignment operator. + /// Move assignment operator. //************************************************************************* list& operator = (list&& rhs) { - if (&rhs != this) - { - this->initialise(); - - typename etl::ilist::iterator itr = rhs.begin(); - while (itr != rhs.end()) - { - this->push_back(etl::move(*itr)); - ++itr; - } - - rhs.initialise(); - } + this->move_container(etl::move(rhs)); return *this; } @@ -2178,7 +2219,7 @@ namespace etl } //************************************************************************* - /// Copy constructor. + /// Copy constructor. Implicit pool. //************************************************************************* list(const list& other) : etl::ilist(*other.p_node_pool, other.p_node_pool->max_size(), true) @@ -2189,26 +2230,35 @@ namespace etl } } + //************************************************************************* + /// Copy constructor. Explicit pool. + //************************************************************************* + list(const list& other, etl::ipool& node_pool) + : etl::ilist(node_pool, node_pool.max_size(), true) + { + if (this != &other) + { + this->assign(other.cbegin(), other.cend()); + } + } + #if ETL_CPP11_SUPPORTED //************************************************************************* - /// Move constructor. + /// Move constructor. Implicit pool. //************************************************************************* list(list&& other) : etl::ilist(*other.p_node_pool, other.p_node_pool->max_size(), true) { - if (this != &other) - { - this->initialise(); + this->move_container(etl::move(other)); + } - typename etl::ilist::iterator itr = other.begin(); - while (itr != other.end()) - { - this->push_back(etl::move(*itr)); - ++itr; - } - - other.initialise(); - } + //************************************************************************* + /// Move constructor. Explicit pool. + //************************************************************************* + list(list&& other, etl::ipool& node_pool) + : etl::ilist(node_pool, node_pool.max_size(), true) + { + this->move_container(etl::move(other)); } #endif @@ -2252,19 +2302,7 @@ namespace etl //************************************************************************* list& operator = (list&& rhs) { - if (&rhs != this) - { - this->initialise(); - - typename etl::ilist::iterator itr = rhs.begin(); - while (itr != rhs.end()) - { - this->push_back(etl::move(*itr)); - ++itr; - } - - rhs.initialise(); - } + this->move_container(etl::move(rhs)); return *this; } @@ -2283,6 +2321,14 @@ namespace etl this->set_node_pool(pool); } + + //************************************************************************* + /// Get the pool instance. + //************************************************************************* + etl::ipool& get_pool() const + { + return *this->p_node_pool; + } }; //************************************************************************* diff --git a/include/etl/version.h b/include/etl/version.h index a9bfc807..73a7b90c 100644 --- a/include/etl/version.h +++ b/include/etl/version.h @@ -39,7 +39,7 @@ SOFTWARE. #define ETL_VERSION_MAJOR 18 #define ETL_VERSION_MINOR 11 -#define ETL_VERSION_PATCH 0 +#define ETL_VERSION_PATCH 1 #define ETL_VERSION ETL_STRINGIFY(ETL_VERSION_MAJOR) "." ETL_STRINGIFY(ETL_VERSION_MINOR) "." ETL_STRINGIFY(ETL_VERSION_PATCH) #define ETL_VERSION_W ETL_STRINGIFY(ETL_VERSION_MAJOR) L"." ETL_STRINGIFY(ETL_VERSION_MINOR) L"." ETL_STRINGIFY(ETL_VERSION_PATCH) #define ETL_VERSION_U16 ETL_STRINGIFY(ETL_VERSION_MAJOR) u"." ETL_STRINGIFY(ETL_VERSION_MINOR) u"." ETL_STRINGIFY(ETL_VERSION_PATCH) diff --git a/library.json b/library.json index 110c098b..79dd239e 100644 --- a/library.json +++ b/library.json @@ -1,6 +1,6 @@ { "name": "Embedded Template Library", - "version": "18.11.0", + "version": "18.11.1", "authors": { "name": "John Wellbelove", "email": "john.wellbelove@etlcpp.com" diff --git a/library.properties b/library.properties index 25846b8f..6f3a4b28 100644 --- a/library.properties +++ b/library.properties @@ -1,5 +1,5 @@ name=Embedded Template Library -version=18.11.0 +version=18.11.1 author= John Wellbelove maintainer=John Wellbelove license=MIT diff --git a/support/Release notes.txt b/support/Release notes.txt index 6360868c..df386dc0 100644 --- a/support/Release notes.txt +++ b/support/Release notes.txt @@ -1,3 +1,7 @@ +=============================================================================== +18.11.1 +Fix and optimise etl::list and etl::forward_list move constructors and assignment operators. + =============================================================================== 18.11.0 Added etl::ibitset::span() member function to return a span of the underlying binary data. diff --git a/test/test_list_shared_pool.cpp b/test/test_list_shared_pool.cpp index 2333d36f..5a557623 100644 --- a/test/test_list_shared_pool.cpp +++ b/test/test_list_shared_pool.cpp @@ -264,7 +264,7 @@ namespace #endif //************************************************************************* - TEST_FIXTURE(SetupFixture, test_copy_constructor) + TEST_FIXTURE(SetupFixture, test_copy_constructor_implicit_pool) { CompareData compare_data(half_data.begin(), half_data.end()); @@ -287,6 +287,82 @@ namespace CHECK_EQUAL(compare_data.size() - 2, other_data.size()); } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_copy_constructor_explicit_pool) + { + CompareData compare_data(half_data.begin(), half_data.end()); + + Pool pool; + DataNDC data(half_data.begin(), half_data.end(), pool); + DataNDC other_data(data, pool); + + CHECK_EQUAL(data.size(), other_data.size()); + + CHECK(std::equal(data.begin(), data.end(), other_data.begin())); + + CHECK(pool.full()); + + other_data.pop_front(); + CHECK_EQUAL(compare_data.size(), data.size()); + CHECK_EQUAL(compare_data.size() - 1, other_data.size()); + + other_data.pop_back(); + CHECK_EQUAL(compare_data.size(), data.size()); + CHECK_EQUAL(compare_data.size() - 2, other_data.size()); + } + + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_move_constructor_implicit_pool) + { + Pool2 pool; + DataNDC data1(sorted_data.begin(), sorted_data.end(), pool); + DataNDC data2(unsorted_data.begin(), unsorted_data.end(), data1.get_pool()); + DataNDC other_data1(std::move(data1)); + DataNDC other_data2(std::move(data2)); + + CHECK_EQUAL(0U, data1.size()); + CHECK(data1.empty()); + CHECK_EQUAL(pool.max_size(), data1.max_size()); + CHECK(data1.begin() == data1.end()); + + CHECK_EQUAL(0U, data2.size()); + CHECK(data2.empty()); + CHECK_EQUAL(pool.max_size(), data2.max_size()); + CHECK(data2.begin() == data2.end()); + + CHECK_EQUAL(sorted_data.size(), other_data1.size()); + CHECK_EQUAL(unsorted_data.size(), other_data2.size()); + + CHECK(std::equal(sorted_data.begin(), sorted_data.end(), other_data1.begin())); + CHECK(std::equal(unsorted_data.begin(), unsorted_data.end(), other_data2.begin())); + } + + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_move_constructor_explicit_pool) + { + Pool2 pool; + DataNDC data1(sorted_data.begin(), sorted_data.end(), pool); + DataNDC data2(unsorted_data.begin(), unsorted_data.end(), data1.get_pool()); + DataNDC other_data1(std::move(data1), pool); + DataNDC other_data2(std::move(data2), pool); + + CHECK_EQUAL(0U, data1.size()); + CHECK(data1.empty()); + CHECK_EQUAL(pool.max_size(), data1.max_size()); + CHECK(data1.begin() == data1.end()); + + CHECK_EQUAL(0U, data2.size()); + CHECK(data2.empty()); + CHECK_EQUAL(pool.max_size(), data2.max_size()); + CHECK(data2.begin() == data2.end()); + + CHECK_EQUAL(sorted_data.size(), other_data1.size()); + CHECK_EQUAL(unsorted_data.size(), other_data2.size()); + + CHECK(std::equal(sorted_data.begin(), sorted_data.end(), other_data1.begin())); + CHECK(std::equal(unsorted_data.begin(), unsorted_data.end(), other_data2.begin())); + } + //************************************************************************* TEST_FIXTURE(SetupFixture, test_iterator) { @@ -1222,6 +1298,38 @@ namespace CHECK(pool.full()); } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_move_assignment) + { + Pool4 pool; + DataNDC data1(sorted_data.begin(), sorted_data.end(), pool); + DataNDC data2(sorted_data.begin(), sorted_data.end(), data1.get_pool()); + DataNDC other_data1(pool); + DataNDC other_data2(pool); + + other_data1 = std::move(data1); + other_data2 = std::move(data2); + + CHECK_EQUAL(0U, data1.size()); + CHECK(data1.empty()); + CHECK_EQUAL(pool.max_size(), data1.max_size()); + CHECK(data1.begin() == data1.end()); + + CHECK_EQUAL(0U, data2.size()); + CHECK(data2.empty()); + CHECK_EQUAL(pool.max_size(), data2.max_size()); + CHECK(data2.begin() == data2.end()); + + CHECK_EQUAL(sorted_data.size(), other_data1.size()); + CHECK_EQUAL(sorted_data.size(), other_data2.size()); + + are_equal = std::equal(sorted_data.begin(), sorted_data.end(), other_data1.begin()); + CHECK(are_equal); + + are_equal = std::equal(sorted_data.begin(), sorted_data.end(), other_data2.begin()); + CHECK(are_equal); + } + //************************************************************************* TEST_FIXTURE(SetupFixture, test_assignment_interface) { @@ -1727,7 +1835,7 @@ namespace to = data.begin(); std::advance(to, 2); - DataNDC data2(data); + DataNDC data2(data, data.get_pool()); data.splice(to, data, begin, end); CHECK_EQUAL(data.size(), data2.size());