From 864eb2a656ecfe1c5a7ba05e7b5bd3b67db66ae5 Mon Sep 17 00:00:00 2001
From: Tom Jackson <tjackson@fb.com>
Date: Fri, 16 Sep 2016 10:09:32 -0700
Subject: [PATCH] Return rvalue references from &&-qualified members of dynamic

Summary: Let the caller do a move, don't force one.

Reviewed By: yfeldblum

Differential Revision: D3873129

fbshipit-source-id: 40c6bf564bcbf794830c99ea1248a9c1bb30e9b0
---
 folly/dynamic-inl.h        |  6 +++---
 folly/dynamic.h            |  6 +++---
 folly/test/DynamicTest.cpp | 13 +++++++++++--
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/folly/dynamic-inl.h b/folly/dynamic-inl.h
index ae484217..909c5b28 100644
--- a/folly/dynamic-inl.h
+++ b/folly/dynamic-inl.h
@@ -437,7 +437,7 @@ inline double&   dynamic::getDouble() & { return get<double>(); }
 inline int64_t&  dynamic::getInt()    & { return get<int64_t>(); }
 inline bool&     dynamic::getBool()   & { return get<bool>(); }
 
-inline std::string dynamic::getString()&& {
+inline std::string&& dynamic::getString()&& {
   return std::move(get<std::string>());
 }
 inline double   dynamic::getDouble() && { return get<double>(); }
@@ -520,7 +520,7 @@ inline dynamic const& dynamic::operator[](dynamic const& idx) const& {
   return at(idx);
 }
 
-inline dynamic dynamic::operator[](dynamic const& idx) && {
+inline dynamic&& dynamic::operator[](dynamic const& idx) && {
   return std::move((*this)[idx]);
 }
 
@@ -549,7 +549,7 @@ inline dynamic& dynamic::at(dynamic const& idx) & {
   return const_cast<dynamic&>(const_cast<dynamic const*>(this)->at(idx));
 }
 
-inline dynamic dynamic::at(dynamic const& idx) && {
+inline dynamic&& dynamic::at(dynamic const& idx) && {
   return std::move(at(idx));
 }
 
diff --git a/folly/dynamic.h b/folly/dynamic.h
index 19b0c2b0..70879759 100644
--- a/folly/dynamic.h
+++ b/folly/dynamic.h
@@ -306,7 +306,7 @@ public:
   double&   getDouble() &;
   int64_t&  getInt() &;
   bool&     getBool() &;
-  std::string getString() &&;
+  std::string&& getString() &&;
   double   getDouble() &&;
   int64_t  getInt() &&;
   bool     getBool() &&;
@@ -383,7 +383,7 @@ public:
    */
   dynamic const& at(dynamic const&) const&;
   dynamic&       at(dynamic const&) &;
-  dynamic        at(dynamic const&) &&;
+  dynamic&&      at(dynamic const&) &&;
 
   /*
    * Like 'at', above, except it returns either a pointer to the contained
@@ -414,7 +414,7 @@ public:
    */
   dynamic&       operator[](dynamic const&) &;
   dynamic const& operator[](dynamic const&) const&;
-  dynamic        operator[](dynamic const&) &&;
+  dynamic&&      operator[](dynamic const&) &&;
 
   /*
    * Only defined for objects, throws TypeError otherwise.
diff --git a/folly/test/DynamicTest.cpp b/folly/test/DynamicTest.cpp
index 624118e4..27d51dc6 100644
--- a/folly/test/DynamicTest.cpp
+++ b/folly/test/DynamicTest.cpp
@@ -408,6 +408,9 @@ TEST(Dynamic, GetString) {
   EXPECT_EQ(s + " hello", d.getString());
 
   EXPECT_EQ(s, std::move(m).getString());
+  EXPECT_EQ(s, m.getString());
+  auto moved = std::move(m).getString();
+  EXPECT_EQ(s, moved);
   EXPECT_NE(dynamic(s), m);
 }
 
@@ -451,7 +454,10 @@ TEST(Dynamic, At) {
   EXPECT_EQ(dynamic(make_long_string() + " hello"), dd.at("key1"));
   EXPECT_EQ(dynamic(make_long_string() + " hello"), dd.at("key1"));
 
-  EXPECT_EQ(ds, std::move(md).at("key1"));
+  EXPECT_EQ(ds, std::move(md).at("key1")); // move available, but not performed
+  EXPECT_EQ(ds, md.at("key1"));
+  dynamic moved = std::move(md).at("key1"); // move performed
+  EXPECT_EQ(ds, moved);
   EXPECT_NE(ds, md.at("key1"));
 }
 
@@ -468,7 +474,10 @@ TEST(Dynamic, Brackets) {
   EXPECT_EQ(dynamic(make_long_string() + " hello"), dd["key1"]);
   EXPECT_EQ(dynamic(make_long_string() + " hello"), dd["key1"]);
 
-  EXPECT_EQ(ds, std::move(md)["key1"]);
+  EXPECT_EQ(ds, std::move(md)["key1"]); // move available, but not performed
+  EXPECT_EQ(ds, md["key1"]);
+  dynamic moved = std::move(md)["key1"]; // move performed
+  EXPECT_EQ(ds, moved);
   EXPECT_NE(ds, md["key1"]);
 }
 
-- 
2.34.1