pass by rvalue in folly::gen::zip and interleave

Summary: Both operators use their own copy of the container (eg line 104), so mutating it should be ok

Reviewed By: ot

Differential Revision: D19170228

fbshipit-source-id: a53094bdbd3fb7b5028cf7ed4e0b073dd919b2d7

folly/gen/Combine-inl.h

@@ -36,7 +36,7 @@ namespace detail {
 template <class Container>
 class Interleave : public Operator<Interleave<Container>> {
   // see comment about copies in CopiedSource
-  const std::shared_ptr<const Container> container_;
+  const std::shared_ptr<Container> container_;
 
  public:
   explicit Interleave(Container container)
@@ -45,30 +45,25 @@ class Interleave : public Operator<Interleave<Container>> {
   template <class Value, class Source>
   class Generator : public GenImpl<Value, Generator<Value, Source>> {
     Source source_;
-    const std::shared_ptr<const Container> container_;
-    typedef const typename Container::value_type& ConstRefType;
-
-    static_assert(
-        std::is_same<const Value&, ConstRefType>::value,
-        "Only matching types may be interleaved");
+    const std::shared_ptr<Container> container_;
 
    public:
     explicit Generator(
         Source source,
-        const std::shared_ptr<const Container> container)
+        const std::shared_ptr<Container> container)
         : source_(std::move(source)), container_(container) {}
 
     template <class Handler>
     bool apply(Handler&& handler) const {
       auto iter = container_->begin();
-      return source_.apply([&](const Value& value) -> bool {
+      return source_.apply([&](Value value) -> bool {
         if (iter == container_->end()) {
           return false;
         }
-        if (!handler(value)) {
+        if (!handler(std::forward<Value>(value))) {
           return false;
         }
-        if (!handler(*iter)) {
+        if (!handler(std::move(*iter))) {
           return false;
         }
         iter++;
@@ -98,38 +93,37 @@ class Interleave : public Operator<Interleave<Container>> {
 template <class Container>
 class Zip : public Operator<Zip<Container>> {
   // see comment about copies in CopiedSource
-  const std::shared_ptr<const Container> container_;
+  const std::shared_ptr<Container> container_;
 
  public:
   explicit Zip(Container container)
       : container_(new Container(std::move(container))) {}
 
   template <
-      class Value1,
+      class Value,
       class Source,
-      class Value2 = decltype(*std::begin(*container_)),
       class Result = std::tuple<
-          typename std::decay<Value1>::type,
-          typename std::decay<Value2>::type>>
-  class Generator
-      : public GenImpl<Result, Generator<Value1, Source, Value2, Result>> {
+          typename std::decay<Value>::type,
+          typename std::decay<typename Container::value_type>::type>>
+  class Generator : public GenImpl<Result, Generator<Value, Source, Result>> {
     Source source_;
-    const std::shared_ptr<const Container> container_;
+    const std::shared_ptr<Container> container_;
 
    public:
     explicit Generator(
         Source source,
-        const std::shared_ptr<const Container> container)
+        const std::shared_ptr<Container> container)
         : source_(std::move(source)), container_(container) {}
 
     template <class Handler>
     bool apply(Handler&& handler) const {
       auto iter = container_->begin();
-      return (source_.apply([&](Value1 value) -> bool {
+      return (source_.apply([&](Value value) -> bool {
         if (iter == container_->end()) {
           return false;
         }
-        if (!handler(std::make_tuple(std::forward<Value1>(value), *iter))) {
+        if (!handler(std::make_tuple(
+                std::forward<Value>(value), std::move(*iter)))) {
           return false;
         }
         ++iter;

folly/gen/test/CombineTest.cpp

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include <memory>
 #include <string>
 #include <tuple>
 #include <vector>
@@ -37,6 +38,13 @@ const folly::gen::detail::Map<folly::gen::detail::MergeTuples> gTupleFlatten{};
 
 auto even = [](int i) -> bool { return i % 2 == 0; };
 auto odd = [](int i) -> bool { return i % 2 == 1; };
+auto initVectorUniquePtr(int value) {
+  std::vector<std::unique_ptr<int>> v;
+  v.push_back(std::make_unique<int>(value));
+  v.push_back(std::make_unique<int>(value));
+  v.push_back(std::make_unique<int>(value));
+  return v;
+}
 
 TEST(CombineGen, Interleave) {
   { // large (infinite) base, small container
@@ -53,6 +61,20 @@ TEST(CombineGen, Interleave) {
   }
 }
 
+TEST(CombineGen, InterleaveMoveOnly) {
+  auto base = initVectorUniquePtr(1);
+  auto toInterleave = initVectorUniquePtr(2);
+  const auto interleaved =
+      from(base) | move | interleave(std::move(toInterleave)) | as<vector>();
+  ASSERT_EQ(interleaved.size(), 6);
+  EXPECT_EQ(*interleaved[0], 1);
+  EXPECT_EQ(*interleaved[1], 2);
+  EXPECT_EQ(*interleaved[2], 1);
+  EXPECT_EQ(*interleaved[3], 2);
+  EXPECT_EQ(*interleaved[4], 1);
+  EXPECT_EQ(*interleaved[5], 2);
+}
+
 TEST(CombineGen, Zip) {
   auto base0 = seq(1);
   // We rely on std::move(fbvector) emptying the source vector
@@ -87,6 +109,20 @@ TEST(CombineGen, Zip) {
   }
 }
 
+TEST(CombineGen, ZipMoveOnly) {
+  auto base = initVectorUniquePtr(1);
+  auto zippee = initVectorUniquePtr(2);
+  const auto combined =
+      from(base) | move | zip(std::move(zippee)) | as<vector>();
+  ASSERT_EQ(combined.size(), 3);
+  EXPECT_EQ(*std::get<0>(combined[0]), 1);
+  EXPECT_EQ(*std::get<1>(combined[0]), 2);
+  EXPECT_EQ(*std::get<0>(combined[1]), 1);
+  EXPECT_EQ(*std::get<1>(combined[1]), 2);
+  EXPECT_EQ(*std::get<0>(combined[2]), 1);
+  EXPECT_EQ(*std::get<1>(combined[2]), 2);
+}
+
 TEST(CombineGen, TupleFlatten) {
   vector<tuple<int, string>> intStringTupleVec{
       tuple<int, string>{1, "1"},