Change the enumerate() example to bind the proxy by reference

Summary:
When compiling without optimizations binding the proxy by
reference is slightly faster (no differences in opt mode), so change
the documentation to recommend this syntax.

The proxy can still be bound by `auto`, `const auto`, and `const
auto&`, in all case behaving as expected and with no overhead in opt
mode. Added a test to make sure these all work.

Reviewed By: yfeldblum, luciang

Differential Revision: D6688958

fbshipit-source-id: 7c6b460a01708786bda7614546fa2e1667f27299

folly/container/Enumerate.h

@@ -29,7 +29,7 @@
  *
  * For example:
  *
- * for (auto it : folly::enumerate(vec)) {
+ * for (auto&& it : folly::enumerate(vec)) {
  *   // *it is a reference to the current element. Const if vec is const.
  *   // it->member can be used as well.
  *   // it.index contains the iteration count.
@@ -37,7 +37,7 @@
  *
  * If the iteration variable is const, the reference is too.
  *
- * for (const auto it : folly::enumerate(vec)) {
+ * for (const auto&& it : folly::enumerate(vec)) {
  *   // *it is always a const reference.
  * }
  *
@@ -125,12 +125,14 @@ class Enumerator {
   }
 
   template <typename OtherIterator>
-  FOLLY_ALWAYS_INLINE bool operator==(const Enumerator<OtherIterator>& rhs) {
+  FOLLY_ALWAYS_INLINE bool operator==(
+      const Enumerator<OtherIterator>& rhs) const {
     return it_ == rhs.it_;
   }
 
   template <typename OtherIterator>
-  FOLLY_ALWAYS_INLINE bool operator!=(const Enumerator<OtherIterator>& rhs) {
+  FOLLY_ALWAYS_INLINE bool operator!=(
+      const Enumerator<OtherIterator>& rhs) const {
     return !(it_ == rhs.it_);
   }
 

folly/container/test/EnumerateTest.cpp

 /*
- * Copyright 2017 Facebook, Inc.
+ * Copyright 2017-present Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,38 +22,6 @@
 #include <folly/container/Enumerate.h>
 #include <folly/portability/GTest.h>
 
-TEST(Enumerate, Basic) {
-  std::vector<std::string> v = {"abc", "a", "ab"};
-  size_t i = 0;
-  for (auto it : folly::enumerate(v)) {
-    EXPECT_EQ(it.index, i);
-    EXPECT_EQ(*it, v[i]);
-    EXPECT_EQ(it->size(), v[i].size());
-
-    // Test mutability.
-    std::string newValue = "x";
-    *it = newValue;
-    EXPECT_EQ(newValue, v[i]);
-
-    ++i;
-  }
-
-  EXPECT_EQ(i, v.size());
-}
-
-TEST(Enumerate, Temporary) {
-  std::vector<std::string> v = {"abc", "a", "ab"};
-  size_t i = 0;
-  for (auto it : folly::enumerate(decltype(v)(v))) { // Copy v.
-    EXPECT_EQ(it.index, i);
-    EXPECT_EQ(*it, v[i]);
-    EXPECT_EQ(it->size(), v[i].size());
-    ++i;
-  }
-
-  EXPECT_EQ(i, v.size());
-};
-
 namespace {
 
 template <class T>
@@ -67,12 +35,57 @@ struct IsConstReference<const T&> {
 
 } // namespace
 
-TEST(Enumerate, BasicConstArg) {
-  const std::vector<std::string> v = {"abc", "a", "ab"};
+#define ENUMERATE_TEST_BASIC(DECL, NAME)             \
+  TEST(Enumerate, NAME) {                            \
+    std::vector<std::string> v = {"abc", "a", "ab"}; \
+    size_t i = 0;                                    \
+    for (DECL it : folly::enumerate(v)) {            \
+      EXPECT_EQ(it.index, i);                        \
+      EXPECT_EQ(*it, v[i]);                          \
+      EXPECT_EQ(it->size(), v[i].size());            \
+                                                     \
+      /* Test mutability. */                         \
+      std::string newValue = "x";                    \
+      *it = newValue;                                \
+      EXPECT_EQ(newValue, v[i]);                     \
+                                                     \
+      ++i;                                           \
+    }                                                \
+                                                     \
+    EXPECT_EQ(i, v.size());                          \
+  }
+
+ENUMERATE_TEST_BASIC(auto, Basic)
+ENUMERATE_TEST_BASIC(auto&&, BasicRRef)
+
+#undef ENUMERATE_TEST_BASIC
+
+#define ENUMERATE_TEST_BASIC_CONST(DECL, NAME)                          \
+  TEST(Enumerate, NAME) {                                               \
+    std::vector<std::string> v = {"abc", "a", "ab"};                    \
+    size_t i = 0;                                                       \
+    for (DECL it : folly::enumerate(v)) {                               \
+      static_assert(                                                    \
+          IsConstReference<decltype(*it)>::value, "Const enumeration"); \
+      EXPECT_EQ(it.index, i);                                           \
+      EXPECT_EQ(*it, v[i]);                                             \
+      EXPECT_EQ(it->size(), v[i].size());                               \
+      ++i;                                                              \
+    }                                                                   \
+                                                                        \
+    EXPECT_EQ(i, v.size());                                             \
+  }
+
+ENUMERATE_TEST_BASIC_CONST(const auto, BasicConst)
+ENUMERATE_TEST_BASIC_CONST(const auto&, BasicConstRef)
+ENUMERATE_TEST_BASIC_CONST(const auto&&, BasicConstRRef)
+
+#undef ENUMERATE_TEST_BASIC_CONST
+
+TEST(Enumerate, Temporary) {
+  std::vector<std::string> v = {"abc", "a", "ab"};
   size_t i = 0;
-  for (auto it : folly::enumerate(v)) {
-    static_assert(
-        IsConstReference<decltype(*it)>::value, "Enumerating a const vector");
+  for (auto&& it : folly::enumerate(decltype(v)(v))) { // Copy v.
     EXPECT_EQ(it.index, i);
     EXPECT_EQ(*it, v[i]);
     EXPECT_EQ(it->size(), v[i].size());
@@ -80,13 +93,14 @@ TEST(Enumerate, BasicConstArg) {
   }
 
   EXPECT_EQ(i, v.size());
-}
+};
 
-TEST(Enumerate, BasicConstEnumerate) {
-  std::vector<std::string> v = {"abc", "a", "ab"};
+TEST(Enumerate, BasicConstArg) {
+  const std::vector<std::string> v = {"abc", "a", "ab"};
   size_t i = 0;
-  for (const auto it : folly::enumerate(v)) {
-    static_assert(IsConstReference<decltype(*it)>::value, "Const enumeration");
+  for (auto&& it : folly::enumerate(v)) {
+    static_assert(
+        IsConstReference<decltype(*it)>::value, "Enumerating a const vector");
     EXPECT_EQ(it.index, i);
     EXPECT_EQ(*it, v[i]);
     EXPECT_EQ(it->size(), v[i].size());
@@ -99,7 +113,7 @@ TEST(Enumerate, BasicConstEnumerate) {
 TEST(Enumerate, TemporaryConstEnumerate) {
   std::vector<std::string> v = {"abc", "a", "ab"};
   size_t i = 0;
-  for (const auto it : folly::enumerate(decltype(v)(v))) { // Copy v.
+  for (const auto&& it : folly::enumerate(decltype(v)(v))) { // Copy v.
     static_assert(IsConstReference<decltype(*it)>::value, "Const enumeration");
     EXPECT_EQ(it.index, i);
     EXPECT_EQ(*it, v[i]);
@@ -113,7 +127,7 @@ TEST(Enumerate, TemporaryConstEnumerate) {
 TEST(Enumerate, RangeSupport) {
   std::vector<std::string> v = {"abc", "a", "ab"};
   size_t i = 0;
-  for (const auto it : folly::enumerate(folly::range(v))) {
+  for (const auto&& it : folly::enumerate(folly::range(v))) {
     EXPECT_EQ(it.index, i);
     EXPECT_EQ(*it, v[i]);
     EXPECT_EQ(it->size(), v[i].size());
@@ -125,7 +139,7 @@ TEST(Enumerate, RangeSupport) {
 
 TEST(Enumerate, EmptyRange) {
   std::vector<std::string> v;
-  for (auto it : folly::enumerate(v)) {
+  for (auto&& it : folly::enumerate(v)) {
     (void)it; // Silence warnings.
     ADD_FAILURE();
   }
@@ -155,13 +169,13 @@ TEST(Enumerate, Cpp17Support) {
   std::array<char, 5> test = {"test"};
   // Can't use range based for loop until C++17, so test manually
   // Equivalent to:
-  // for (const auto it : folly::enumerate(CStringRange{test.data()})) { ... }
+  // for (const auto&& it : folly::enumerate(CStringRange{test.data()})) { ... }
   {
     auto&& enumerate = folly::enumerate(CStringRange{test.data()});
     auto begin = enumerate.begin();
     auto end = enumerate.end();
     for (; begin != end; ++begin) {
-      const auto it = *begin;
+      const auto&& it = *begin;
 
       ASSERT_LT(it.index, test.size());
       EXPECT_EQ(*it, test[it.index]);

folly/container/test/ForeachBenchmark.cpp

@@ -389,7 +389,7 @@ BENCHMARK(CharVecForRangeEnumerate, iters) {
     setupCharVecBenchmark(iters);
   }
   size_t sum = 0;
-  for (auto it : enumerate(vec_char)) {
+  for (auto&& it : enumerate(vec_char)) {
     sum += *it * it.index;
   }
   doNotOptimizeAway(sum);