support operator-> (#2051)

Summary:
Pull Request resolved: #2051

After experimenting a little, I have to say that not having `operator->` harms usability a lot.

Here is how we can do it: https://quuxplusone.github.io/blog/2019/02/06/arrow-proxy/

Reviewed By: dmm-fb

Differential Revision: D48032674

fbshipit-source-id: 0e7aad679d74545cd59054688d461a98194f65ad

folly/container/Iterator.h

@@ -583,6 +583,20 @@ back_emplace_iterator<Container, implicit_unpack> back_emplacer(Container& c) {
   return back_emplace_iterator<Container, implicit_unpack>(c);
 }
 
+namespace detail {
+
+// An accepted way to make operator-> work
+// https://quuxplusone.github.io/blog/2019/02/06/arrow-proxy/
+template <typename Ref>
+struct arrow_proxy {
+  Ref res;
+  Ref* operator->() { return &res; }
+
+  explicit arrow_proxy(Ref* ref) : res(*ref) {}
+};
+
+} // namespace detail
+
 /**
  * index_iterator
  *
@@ -634,7 +648,11 @@ class index_iterator {
   using reference = decltype(FOLLY_DECLVAL(container_type&)[size_type{}]);
   using difference_type =
       detected_or_t<std::ptrdiff_t, get_difference_type_t, Container>;
-  // no pointer type - in C++20 not needed and can be difficult.
+
+  using pointer = std::conditional_t<
+      std::is_reference<reference>::value,
+      std::remove_reference_t<reference>*,
+      detail::arrow_proxy<reference>>;
 
   static_assert(
       std::is_signed<difference_type>::value, "difference_type must be signed");
@@ -665,8 +683,15 @@ class index_iterator {
 
   constexpr reference operator*() const { return (*container_)[index_]; }
 
-  // no operator->. It is doable but not required in C++20 and up
-  // and is quite difficult for proxies. ranges forego operator-> and so do we
+  pointer operator->() const {
+    // It's equivalent to pointer{&**this} but compiler stops
+    // compilation on taking an address of a temporary.
+    // In this case `arrow_proxy` will copy the temporary and there is no
+    // issue.
+    auto&& ref = **this;
+    pointer res{&ref};
+    return res;
+  }
 
   constexpr reference operator[](difference_type n) const {
     return *(*this + n);

folly/container/test/IteratorTest.cpp

@@ -844,6 +844,12 @@ TEST(IndexIterator, UseProxyReferences) {
   static_assert(std::is_same<it_ref_t, std::pair<int, int&>>::value, "");
   static_assert(std::is_same<it_cref_t, std::pair<int, const int&>>::value, "");
 
+  static_assert(std::is_same<decltype(it {} -> first), int>::value, "");
+  static_assert(std::is_same<decltype(it {} -> second), int&>::value, "");
+  static_assert(std::is_same<decltype(cit {} -> first), int>::value, "");
+  static_assert(
+      std::is_same<decltype(cit {} -> second), const int&>::value, "");
+
   ASSERT_EQ(4, (std::count_if(civ.begin(), civ.end(), [](auto&& pair) {
               return pair.first == pair.second;
             })));
@@ -862,4 +868,32 @@ TEST(IndexIterator, UseProxyReferences) {
 
   std::vector<int> expected{-1, -1, -1, -1};
   ASSERT_EQ(expected, iv.v_);
+
+  // testing pointer proxies
+  for (auto f = iv.begin(); f != iv.end(); ++f) {
+    f->second = 1;
+  }
+
+  expected = {1, 1, 1, 1};
+  ASSERT_EQ(expected, iv.v_);
+}
+
+TEST(IndexIterator, OperatorArrowForNonProxies) {
+  using v_t = std::vector<std::array<int, 2>>;
+  using iterator = folly::index_iterator<v_t>;
+
+  static_assert(std::is_same<iterator::pointer, v_t::pointer>::value, "");
+
+  v_t v;
+  v.resize(3);
+  iterator f{v, 0};
+  iterator l{v, v.size()};
+
+  f->at(0) = 1;
+  (f + 1)->at(0) = 2;
+  (f + 2)->at(0) = 3;
+
+  v_t expected{{1, 0}, {2, 0}, {3, 0}};
+
+  ASSERT_EQ(expected, v);
 }