Improve iterator_range to take non-&& arguments

The current implementation takes forwarding reference arguments, which
is fine when you call it with rvalues, like
make_iterator_range(V.begin(), V.end()). If you call it with lvalues
though, it doesn't do what you'd expect. ForwardIterator becomes a
reference:

Foo I = V.begin();
make_iterator_range(I, I); //ForwardIterator is deduced as Foo&

Since iterator are supposed to be small, no harm in passing them by
value.

Change-Id: I151c87304949d810c72c42f60e9d1a7151f61f83
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2020780
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66007}

src/base/iterator.h

@@ -37,9 +37,8 @@ class iterator_range {
 
   iterator_range() : begin_(), end_() {}
   template <typename ForwardIterator1, typename ForwardIterator2>
-  iterator_range(ForwardIterator1&& begin, ForwardIterator2&& end)
-      : begin_(std::forward<ForwardIterator1>(begin)),
-        end_(std::forward<ForwardIterator2>(end)) {}
+  iterator_range(ForwardIterator1 begin, ForwardIterator2 end)
+      : begin_(begin), end_(end) {}
 
   iterator begin() { return begin_; }
   iterator end() { return end_; }
@@ -60,9 +59,8 @@ class iterator_range {
 };
 
 template <typename ForwardIterator>
-auto make_iterator_range(ForwardIterator&& begin, ForwardIterator&& end) {
-  return iterator_range<ForwardIterator>{std::forward<ForwardIterator>(begin),
-                                         std::forward<ForwardIterator>(end)};
+auto make_iterator_range(ForwardIterator begin, ForwardIterator end) {
+  return iterator_range<ForwardIterator>{begin, end};
 }
 
 // {Reversed} returns a container adapter usable in a range-based "for"

test/unittests/base/iterator-unittest.cc

@@ -20,7 +20,6 @@ TEST(IteratorTest, IteratorRangeEmpty) {
   EXPECT_EQ(0, r.size());
 }
 
-
 TEST(IteratorTest, IteratorRangeArray) {
   int array[10] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
   base::iterator_range<int*> r1(&array[0], &array[10]);
@@ -40,7 +39,6 @@ TEST(IteratorTest, IteratorRangeArray) {
   }
 }
 
-
 TEST(IteratorTest, IteratorRangeDeque) {
   using C = std::deque<int>;
   C c;
@@ -57,5 +55,18 @@ TEST(IteratorTest, IteratorRangeDeque) {
   EXPECT_EQ(2, std::count(r.begin(), r.end(), 2));
 }
 
+TEST(IteratorTest, IteratorTypeDeduction) {
+  base::iterator_range<char*> r;
+  auto r2 = make_iterator_range(r.begin(), r.end());
+  EXPECT_EQ(r2.begin(), r.begin());
+  EXPECT_EQ(r2.end(), r2.end());
+  auto I = r.begin(), E = r.end();
+  // Check that this compiles and does the correct thing even if the iterators
+  // are lvalues:
+  auto r3 = make_iterator_range(I, E);
+  EXPECT_TRUE((std::is_same<decltype(r2), decltype(r3)>::value));
+  EXPECT_EQ(r3.begin(), r.begin());
+  EXPECT_EQ(r3.end(), r2.end());
+}
 }  // namespace base
 }  // namespace v8