Merge pull request #21 from leg0/proper_string_move

Make String move constructor move instead of copy.

Author: aentinger

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

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)

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

test/src/String/test_move.cpp

@@ -0,0 +1,37 @@
+#include <catch.hpp>
+
+#include <String.h>
+
+#include "StringPrinter.h"
+
+#include <utility>
+
+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");
+}