Value: Add a specialized rvalue reference of Get()

The move `String(Value&&)` constructor tries to partially move `String`
values from a `Value` type. However, since there was no an appropriate
`Value::Get<T>()` implementation that binds to the requested move
operation, the compiler will actually not move the value but copy it
instead as the only available implementation of `Value::Get<T>()`
returns a const reference `const T&`. This commit adds a new overload
that returns a non-const reference and allows to optionally move the string
value of a Value type.

lib/base/string.cpp

@@ -47,7 +47,7 @@ String::String(Value&& other)
 String& String::operator=(Value&& other)
 {
 	if (other.IsString())
-		m_Data = std::move(other.Get<String>());
+		*this = std::move(other.Get<String>()); // Will atomically bind to the move assignment operator below.
 	else
 		*this = static_cast<String>(other);
 

lib/base/value.cpp

@@ -9,9 +9,13 @@ using namespace icinga;
 
 template class boost::variant<boost::blank, double, bool, String, Object::Ptr>;
 template const double& Value::Get<double>() const;
+template double& Value::Get<double>();
 template const bool& Value::Get<bool>() const;
+template bool& Value::Get<bool>();
 template const String& Value::Get<String>() const;
+template String& Value::Get<String>();
 template const Object::Ptr& Value::Get<Object::Ptr>() const;
+template Object::Ptr& Value::Get<Object::Ptr>();
 
 const Value icinga::Empty;
 

lib/base/value.hpp

@@ -140,14 +140,24 @@ public:
 		return boost::get<T>(m_Value);
 	}
 
+	template<typename T>
+	T& Get()
+	{
+		return boost::get<T>(m_Value);
+	}
+
 private:
 	boost::variant<boost::blank, double, bool, String, Object::Ptr> m_Value;
 };
 
 extern template const double& Value::Get<double>() const;
+extern template double& Value::Get<double>();
 extern template const bool& Value::Get<bool>() const;
+extern template bool& Value::Get<bool>();
 extern template const String& Value::Get<String>() const;
+extern template String& Value::Get<String>();
 extern template const Object::Ptr& Value::Get<Object::Ptr>() const;
+extern template Object::Ptr& Value::Get<Object::Ptr>();
 
 extern const Value Empty;
 

test/CMakeLists.txt

@@ -166,6 +166,7 @@ add_boost_test(base
     base_string/index
     base_string/find
     base_string/vector_move
+    base_string/move_string_out_of_Value_type
     base_timer/construct
     base_timer/interval
     base_timer/invoke

test/base-string.cpp

@@ -1,6 +1,7 @@
 /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */
 
 #include "base/string.hpp"
+#include "base/value.hpp"
 #include <vector>
 #include <BoostTestTargetConfig.h>
 
@@ -126,4 +127,20 @@ BOOST_AUTO_TEST_CASE(vector_move)
 	BOOST_CHECK_EQUAL(oldAddr, newAddr);
 }
 
+// Test that the move constructor of icinga::String actually moves the underlying std::string out of a Value instance.
+// The constructor overload is only available on non-Windows platforms though, so we need to skip the test on Windows.
+BOOST_AUTO_TEST_CASE(move_string_out_of_Value_type)
+{
+#ifndef _MSC_VER
+	Value value("Icinga 2");
+	String other = value.Get<String>(); // We didn't request a move, so this should just copy the string.
+	BOOST_CHECK_EQUAL("Icinga 2", value.Get<String>());
+	BOOST_CHECK_EQUAL("Icinga 2", other);
+
+	String newStr = std::move(value);
+	BOOST_CHECK_EQUAL("", value.Get<String>());
+	BOOST_CHECK_EQUAL(newStr, "Icinga 2");
+#endif /* _MSC_VER */
+}
+
 BOOST_AUTO_TEST_SUITE_END()