commit 9a9d46f6b6a09607c00bef0f85d302f2f70759a4
author Ben Clayton <headlessclayton@gmail.com> Fri Jan 24 15:03:29 2020 +0000
committer Ben Clayton <bclayton@google.com> Fri Jan 24 15:21:26 2020 +0000
tree 7359cba22ca68fed944baa53d14f50cb4d6c1d87
parent 3e6cfd6c2f174ac79835015d1e1fe35fc1c008b7

Fix bad usage of std::move

Issue #16 describes the problem and solution perfectly.

Updated tests to cover this.

Fixes #16

include/dap/optional.h

diff --git a/include/dap/optional.h b/include/dap/optional.h
index 06b298c..7e1d283 100644
--- a/include/dap/optional.h
+++ b/include/dap/optional.h
@@ -17,7 +17,7 @@
 
 #include <assert.h>
 #include <type_traits>
-#include <utility>  // std::move
+#include <utility>  // std::move, std::forward
 
 namespace dap {
 
@@ -104,7 +104,7 @@
 
 template <typename T>
 template <typename U /*= T*/, typename>
-optional<T>::optional(U&& value) : val(std::move(value)), set(true) {}
+optional<T>::optional(U&& value) : val(std::forward<U>(value)), set(true) {}
 
 template <typename T>
 T& optional<T>::value() {
@@ -153,7 +153,7 @@
 template <typename T>
 template <typename U /* = T */, typename>
 optional<T>& optional<T>::operator=(U&& value) {
-  val = std::move(value);
+  val = std::forward<U>(value);
   set = true;
   return *this;
 }

src/optional_test.cpp

diff --git a/src/optional_test.cpp b/src/optional_test.cpp
index 4d78379..b2590fc 100644
--- a/src/optional_test.cpp
+++ b/src/optional_test.cpp
@@ -17,23 +17,27 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include <string>
+
 TEST(Optional, EmptyConstruct) {
-  dap::optional<int> opt;
+  dap::optional<std::string> opt;
   ASSERT_FALSE(opt);
   ASSERT_FALSE(opt.has_value());
 }
 
 TEST(Optional, ValueConstruct) {
-  dap::optional<int> opt(0);
+  dap::optional<int> opt(10);
   ASSERT_TRUE(opt);
   ASSERT_TRUE(opt.has_value());
+  ASSERT_EQ(opt.value(), 10);
 }
 
 TEST(Optional, CopyConstruct) {
-  dap::optional<int> a(10);
-  dap::optional<int> b(a);
+  dap::optional<std::string> a("meow");
+  dap::optional<std::string> b(a);
   ASSERT_EQ(a, b);
-  ASSERT_EQ(b.value(), 10);
+  ASSERT_EQ(a.value(), "meow");
+  ASSERT_EQ(b.value(), "meow");
 }
 
 TEST(Optional, CopyCastConstruct) {
@@ -44,9 +48,9 @@
 }
 
 TEST(Optional, MoveConstruct) {
-  dap::optional<int> a(10);
-  dap::optional<int> b(std::move(a));
-  ASSERT_EQ(b.value(), 10);
+  dap::optional<std::string> a("meow");
+  dap::optional<std::string> b(std::move(a));
+  ASSERT_EQ(b.value(), "meow");
 }
 
 TEST(Optional, MoveCastConstruct) {
@@ -56,33 +60,36 @@
 }
 
 TEST(Optional, AssignValue) {
-  dap::optional<int> a;
-  a = 10;
-  ASSERT_EQ(a.value(), 10);
+  dap::optional<std::string> a;
+  std::string b = "meow";
+  a = b;
+  ASSERT_EQ(a.value(), "meow");
+  ASSERT_EQ(b, "meow");
 }
 
 TEST(Optional, AssignOptional) {
-  dap::optional<int> a;
-  dap::optional<int> b(10);
+  dap::optional<std::string> a;
+  dap::optional<std::string> b("meow");
   a = b;
-  ASSERT_EQ(a.value(), 10);
+  ASSERT_EQ(a.value(), "meow");
+  ASSERT_EQ(b.value(), "meow");
 }
 
 TEST(Optional, MoveAssignOptional) {
-  dap::optional<int> a;
-  dap::optional<int> b(10);
+  dap::optional<std::string> a;
+  dap::optional<std::string> b("meow");
   a = std::move(b);
-  ASSERT_EQ(a.value(), 10);
+  ASSERT_EQ(a.value(), "meow");
 }
 
 TEST(Optional, StarDeref) {
-  dap::optional<int> a(10);
-  ASSERT_EQ(*a, 10);
+  dap::optional<std::string> a("meow");
+  ASSERT_EQ(*a, "meow");
 }
 
 TEST(Optional, StarDerefConst) {
-  const dap::optional<int> a(10);
-  ASSERT_EQ(*a, 10);
+  const dap::optional<std::string> a("meow");
+  ASSERT_EQ(*a, "meow");
 }
 
 TEST(Optional, ArrowDeref) {
@@ -102,15 +109,15 @@
 }
 
 TEST(Optional, Value) {
-  const dap::optional<int> a(10);
-  ASSERT_EQ(a.value(), 10);
+  const dap::optional<std::string> a("meow");
+  ASSERT_EQ(a.value(), "meow");
 }
 
 TEST(Optional, ValueDefault) {
-  const dap::optional<int> a;
-  const dap::optional<int> b(20);
-  ASSERT_EQ(a.value(10), 10);
-  ASSERT_EQ(b.value(10), 20);
+  const dap::optional<std::string> a;
+  const dap::optional<std::string> b("woof");
+  ASSERT_EQ(a.value("meow"), "meow");
+  ASSERT_EQ(b.value("meow"), "woof");
 }
 
 TEST(Optional, CompareLT) {