change F14*Set::eraseInto callback arg from lvalue to rvalue

Summary:
The value passed to the eraseInto beforeDestroy callback will
be immediately destroyed, so it makes more sense for it to be an rvalue
reference than an lvalue reference.

Reviewed By: yfeldblum, shixiao

Differential Revision: D7986157

fbshipit-source-id: 9dcc5371635a860033fe901aaad3a73230d652f6

folly/container/F14Set.h

@@ -393,19 +393,19 @@ class F14BasicSet {
   }
 
   FOLLY_ALWAYS_INLINE iterator erase(const_iterator pos) {
-    return eraseInto(pos, [](value_type&) {});
+    return eraseInto(pos, [](value_type&&) {});
   }
 
   iterator erase(const_iterator first, const_iterator last) {
-    return eraseInto(first, last, [](value_type&) {});
+    return eraseInto(first, last, [](value_type&&) {});
   }
 
   size_type erase(key_type const& key) {
-    return eraseInto(key, [](value_type&) {});
+    return eraseInto(key, [](value_type&&) {});
   }
 
   // eraseInto contains the same overloads as erase but provides
-  // an additional callback argument which is called with a non-const lvalue
+  // an additional callback argument which is called with an rvalue
   // reference to the item directly before it is destroyed. This can be
   // used to extract an item out of a F14Set while avoiding a copy.
   template <typename BeforeDestroy>
@@ -865,15 +865,15 @@ class F14VectorSet
 
  public:
   FOLLY_ALWAYS_INLINE iterator erase(const_iterator pos) {
-    return eraseInto(pos, [](value_type&) {});
+    return eraseInto(pos, [](value_type&&) {});
   }
 
   iterator erase(const_iterator first, const_iterator last) {
-    return eraseInto(first, last, [](value_type&) {});
+    return eraseInto(first, last, [](value_type&&) {});
   }
 
   std::size_t erase(key_type const& key) {
-    return eraseInto(key, [](value_type&) {});
+    return eraseInto(key, [](value_type&&) {});
   }
 
   template <typename BeforeDestroy>

folly/container/detail/F14Policy.h

@@ -408,8 +408,8 @@ class ValueContainerPolicy : public BasePolicy<
     return item;
   }
 
-  Value& valueAtItemForExtract(Item& item) {
-    return item;
+  Value&& valueAtItemForMove(Item& item) {
+    return std::move(item);
   }
 
   template <typename... Args>
@@ -656,8 +656,8 @@ class NodeContainerPolicy
     return *item;
   }
 
-  Value& valueAtItemForExtract(Item& item) {
-    return *item;
+  Value&& valueAtItemForMove(Item& item) {
+    return std::move(*item);
   }
 
   template <typename... Args>
@@ -951,8 +951,8 @@ class VectorContainerPolicy : public BasePolicy<
     return {item};
   }
 
-  Value& valueAtItemForExtract(Item& item) {
-    return values_[item];
+  Value&& valueAtItemForMove(Item& item) {
+    return std::move(values_[item]);
   }
 
   void constructValueAtItem(

folly/container/detail/F14Table.h

@@ -1814,7 +1814,7 @@ class F14Table : public Policy {
   // to intercept the value before it is destroyed (to extract it, for
   // example), use eraseInto(pos, beforeDestroy).
   void erase(ItemIter pos) {
-    eraseInto(pos, [](value_type&) {});
+    eraseInto(pos, [](value_type&&) {});
   }
 
   // The item needs to still be hashable during this call.  If you want
@@ -1826,13 +1826,13 @@ class F14Table : public Policy {
     if (pos.chunk()->hostedOverflowCount() != 0) {
       hp = splitHash(this->computeItemHash(pos.citem()));
     }
-    beforeDestroy(this->valueAtItemForExtract(pos.item()));
+    beforeDestroy(this->valueAtItemForMove(pos.item()));
     eraseImpl(pos, hp);
   }
 
   template <typename K>
   std::size_t erase(K const& key) {
-    return eraseInto(key, [](value_type&) {});
+    return eraseInto(key, [](value_type&&) {});
   }
 
   template <typename K, typename BeforeDestroy>
@@ -1843,7 +1843,7 @@ class F14Table : public Policy {
     auto hp = splitHash(this->computeKeyHash(key));
     auto iter = findImpl(hp, key);
     if (!iter.atEnd()) {
-      beforeDestroy(this->valueAtItemForExtract(iter.item()));
+      beforeDestroy(this->valueAtItemForMove(iter.item()));
       eraseImpl(iter, hp);
       return 1;
     } else {

folly/container/test/F14SetTest.cpp

@@ -723,11 +723,13 @@ void runEraseIntoTest() {
   S t0;
   S t1;
 
-  auto insertIntoT0 = [&t0](auto& value) {
+  auto insertIntoT0 = [&t0](auto&& value) {
     EXPECT_FALSE(value.destroyed);
     t0.emplace(std::move(value));
   };
-  auto insertIntoT0Mut = [&](auto& value) mutable { insertIntoT0(value); };
+  auto insertIntoT0Mut = [&](typename S::value_type&& value) mutable {
+    insertIntoT0(std::move(value));
+  };
 
   t0.insert(10);
   t1.insert(20);