Forwards key to Group

If we knew the key type was not a reference, we could just call
std::move, but there might be an lvalue reference hiding in there.

Author: Ryan Haining

cppitertools/groupby.hpp

@@ -178,6 +178,8 @@ class iter::impl::GroupProducer {
     friend class Iterator;
     friend class GroupIterator;
     Iterator<ContainerT>& owner_;
+    // The key function may return a reference, so we need to call forward, not
+    // move, when going for efficiency.
     key_func_ret<ContainerT> key_;
 
     // completed is set if a Group is iterated through
@@ -192,7 +194,7 @@ class iter::impl::GroupProducer {
     bool completed = false;
 
     Group(Iterator<ContainerT>& owner, key_func_ret<ContainerT> key)
-        : owner_(owner), key_(key) {}
+        : owner_(owner), key_(std::forward<key_func_ret<ContainerT>>(key)) {}
 
    public:
     ~Group() {
@@ -204,7 +206,9 @@ class iter::impl::GroupProducer {
 
     // move-constructible, non-copy-constructible, non-assignable
     Group(Group&& other) noexcept
-        : owner_(other.owner_), key_{other.key_}, completed{other.completed} {
+        : owner_(other.owner_),
+          key_{std::forward<key_func_ret<ContainerT>>(other.key_)},
+          completed{other.completed} {
       other.completed = true;
     }
 

test/test_groupby.cpp

@@ -40,6 +40,43 @@ namespace {
 
   const std::vector<std::string> vec = {
       "hi", "ab", "ho", "abc", "def", "abcde", "efghi"};
+
+  struct Person {
+    std::string name;
+    int id;
+    bool operator==(const Person& other) const {
+      return id == other.id;
+    }
+  };
+
+  std::string& get_name(Person& p) {
+    return p.name;
+  }
+
+  template <typename G>
+  std::vector<Person> extract_person_group(G g) {
+    return {std::begin(g), std::end(g)};
+  }
+}
+
+TEST_CASE("groupby: handle key function that returns reference", "[groupby]") {
+  std::vector<Person> people = {{"first", 1}, {"first", 2}, {"first", 3}};
+  std::vector<std::string> keys;
+  std::vector<std::vector<Person>> groups;
+
+  for (auto&& gb : groupby(people, get_name)) {
+    groups.push_back(extract_person_group(std::move(gb.second)));
+    keys.push_back(gb.first);
+  }
+
+  const std::vector<std::string> kc = {"first"};
+  const std::vector<std::vector<Person>> gc = {
+      {{"first", 1}, {"first", 2}, {"first", 3}}};
+
+  REQUIRE(people[0].name == "first");
+  REQUIRE(gc[0][0].name == "first");
+  REQUIRE(keys == kc);
+  REQUIRE(groups == gc);
 }
 
 TEST_CASE("groupby: handles different callable types", "[groupby]") {