From 9668aa626d6f1f3427b88aed06175bc312a1ce84 Mon Sep 17 00:00:00 2001 From: Ahti Legonkov Date: Thu, 15 Nov 2018 23:16:49 +0200 Subject: [PATCH 1/2] Make String move constructor move instead of copy. The move constructor String::String(String&&) and String::operator=(String&&) now perform move instead of copy. Remove String(StringSumHelper&&) constructor because having it makes no sense: String(String&&) takes care of it - you can pass either String&& or StringSumHelper&& to this constructor. StringSumHelper is derived from String and has no data members other than those inherited from String. Even if it did have some extra data members, truncation would have to happen during move, and normally that is something you don't want. --- api/String.cpp | 48 ++++++++++++++++++------------------------------ api/String.h | 2 -- 2 files changed, 18 insertions(+), 32 deletions(-) diff --git a/api/String.cpp b/api/String.cpp index 1b6d4b28..6cf244ac 100644 --- a/api/String.cpp +++ b/api/String.cpp @@ -65,14 +65,13 @@ String::String(const __FlashStringHelper *pstr) #if __cplusplus >= 201103L || defined(__GXX_EXPERIMENTAL_CXX0X__) String::String(String &&rval) + : buffer(rval.buffer) + , capacity(rval.capacity) + , len(rval.len) { - init(); - move(rval); -} -String::String(StringSumHelper &&rval) -{ - init(); - move(rval); + rval.buffer = NULL; + rval.capacity = 0; + rval.len = 0; } #endif @@ -217,23 +216,18 @@ String & String::copy(const __FlashStringHelper *pstr, unsigned int length) #if __cplusplus >= 201103L || defined(__GXX_EXPERIMENTAL_CXX0X__) void String::move(String &rhs) { - if (buffer) { - if (rhs && capacity >= rhs.len) { - memcpy(buffer, rhs.buffer, rhs.len); - len = rhs.len; - buffer[len] = '\0'; - rhs.len = 0; - return; - } else { - free(buffer); - } + if (this != &rhs) + { + free(buffer); + + buffer = rhs.buffer; + len = rhs.len; + capacity = rhs.capacity; + + rhs.buffer = NULL; + rhs.len = 0; + rhs.capacity = 0; } - buffer = rhs.buffer; - capacity = rhs.capacity; - len = rhs.len; - rhs.buffer = NULL; - rhs.capacity = 0; - rhs.len = 0; } #endif @@ -250,13 +244,7 @@ String & String::operator = (const String &rhs) #if __cplusplus >= 201103L || defined(__GXX_EXPERIMENTAL_CXX0X__) String & String::operator = (String &&rval) { - if (this != &rval) move(rval); - return *this; -} - -String & String::operator = (StringSumHelper &&rval) -{ - if (this != &rval) move(rval); + move(rval); return *this; } #endif diff --git a/api/String.h b/api/String.h index 3f34493b..c4d11bcc 100644 --- a/api/String.h +++ b/api/String.h @@ -74,7 +74,6 @@ class String String(const __FlashStringHelper *str); #if __cplusplus >= 201103L || defined(__GXX_EXPERIMENTAL_CXX0X__) String(String &&rval); - String(StringSumHelper &&rval); #endif explicit String(char c); explicit String(unsigned char, unsigned char base=10); @@ -101,7 +100,6 @@ class String String & operator = (const __FlashStringHelper *str); #if __cplusplus >= 201103L || defined(__GXX_EXPERIMENTAL_CXX0X__) String & operator = (String &&rval); - String & operator = (StringSumHelper &&rval); #endif // concatenate (works w/ built-in types) From 41599a304f19ddf52707a5062012651d250acd7b Mon Sep 17 00:00:00 2001 From: Ahti Legonkov Date: Tue, 20 Apr 2021 23:49:46 +0300 Subject: [PATCH 2/2] Add tests for String move --- test/CMakeLists.txt | 1 + test/src/String/test_move.cpp | 37 +++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 test/src/String/test_move.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 65e15c47..f731b577 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -66,6 +66,7 @@ set(TEST_SRCS src/String/test_indexOf.cpp src/String/test_lastIndexOf.cpp src/String/test_length.cpp + src/String/test_move.cpp src/String/test_remove.cpp src/String/test_replace.cpp src/String/test_String.cpp diff --git a/test/src/String/test_move.cpp b/test/src/String/test_move.cpp new file mode 100644 index 00000000..d37630c4 --- /dev/null +++ b/test/src/String/test_move.cpp @@ -0,0 +1,37 @@ +#include + +#include + +#include "StringPrinter.h" + +#include + +TEST_CASE("Testing String move constructor", "[String-move-01]") +{ + arduino::String a("src"); + char const* const a_str = a.c_str(); + arduino::String b(std::move(a)); + REQUIRE(a.length() == 0); + REQUIRE(a.c_str() == nullptr); + REQUIRE(b.c_str() == a_str); + REQUIRE(b.length() == 3); +} + +TEST_CASE("Testing String move assignment", "[String-move-02]") +{ + arduino::String a("src"); + char const* const a_str = a.c_str(); + arduino::String b; + b = std::move(a); + REQUIRE(a.length() == 0); + REQUIRE(a.c_str() == nullptr); + REQUIRE(b == arduino::String("src")); + REQUIRE(b.c_str() == a_str); +} + +TEST_CASE("Testing String move self assignment", "[String-move-03]") +{ + arduino::String a("src"); + a = std::move(a); + REQUIRE(a == "src"); +}