Non-const `dynamic` iterators

Summary:
Add non-const iterators corresponding to existing iterator types
where it makes sense.

Reviewed By: yfeldblum

Differential Revision: D4499520

fbshipit-source-id: dc4ec583d3da1c6701805b30b25389fbf9311a7c

diff --git a/folly/dynamic-inl.h b/folly/dynamic-inl.h
index 41352ad070ac563ac7976a2c006a4065a9ec4e6c..dbe36b5d9347314dfe479e9d9415ff5335156213 100644 (file)
--- a/folly/dynamic-inl.h
+++ b/folly/dynamic-inl.h
@@ -174,10 +174,42 @@ inline dynamic::ObjectMaker dynamic::object(dynamic a, dynamic b) {
 
 //////////////////////////////////////////////////////////////////////
 
+struct dynamic::item_iterator : boost::iterator_adaptor<
+                                    dynamic::item_iterator,
+                                    dynamic::ObjectImpl::iterator> {
+  /* implicit */ item_iterator(base_type b) : iterator_adaptor_(b) {}
+
+  using object_type = dynamic::ObjectImpl;
+
+ private:
+  friend class boost::iterator_core_access;
+};
+
+struct dynamic::value_iterator : boost::iterator_adaptor<
+                                     dynamic::value_iterator,
+                                     dynamic::ObjectImpl::iterator,
+                                     dynamic> {
+  /* implicit */ value_iterator(base_type b) : iterator_adaptor_(b) {}
+
+  using object_type = dynamic::ObjectImpl;
+
+ private:
+  dynamic& dereference() const {
+    return base_reference()->second;
+  }
+  friend class boost::iterator_core_access;
+};
+
 struct dynamic::const_item_iterator
   : boost::iterator_adaptor<dynamic::const_item_iterator,
                             dynamic::ObjectImpl::const_iterator> {
   /* implicit */ const_item_iterator(base_type b) : iterator_adaptor_(b) { }
+  /* implicit */ const_item_iterator(item_iterator i)
+      : iterator_adaptor_(i.base()) {}
+  /* implicit */ const_item_iterator(dynamic::ObjectImpl::iterator i)
+      : iterator_adaptor_(i) {}
+
+  using object_type = dynamic::ObjectImpl const;
 
  private:
   friend class boost::iterator_core_access;
@@ -189,6 +221,8 @@ struct dynamic::const_key_iterator
                             dynamic const> {
   /* implicit */ const_key_iterator(base_type b) : iterator_adaptor_(b) { }
 
+  using object_type = dynamic::ObjectImpl const;
+
  private:
   dynamic const& dereference() const {
     return base_reference()->first;
@@ -201,6 +235,12 @@ struct dynamic::const_value_iterator
                             dynamic::ObjectImpl::const_iterator,
                             dynamic const> {
   /* implicit */ const_value_iterator(base_type b) : iterator_adaptor_(b) { }
+  /* implicit */ const_value_iterator(value_iterator i)
+      : iterator_adaptor_(i.base()) {}
+  /* implicit */ const_value_iterator(dynamic::ObjectImpl::iterator i)
+      : iterator_adaptor_(i) {}
+
+  using object_type = dynamic::ObjectImpl const;
 
  private:
   dynamic const& dereference() const {
@@ -307,12 +347,20 @@ inline dynamic::const_iterator dynamic::end() const {
   return get<Array>().end();
 }
 
+inline dynamic::iterator dynamic::begin() {
+  return get<Array>().begin();
+}
+inline dynamic::iterator dynamic::end() {
+  return get<Array>().end();
+}
+
 template <class It>
 struct dynamic::IterableProxy {
-  typedef It const_iterator;
+  typedef It iterator;
   typedef typename It::value_type value_type;
+  typedef typename It::object_type object_type;
 
-  /* implicit */ IterableProxy(const dynamic::ObjectImpl* o) : o_(o) { }
+  /* implicit */ IterableProxy(object_type* o) : o_(o) {}
 
   It begin() const {
     return o_->begin();
@@ -323,7 +371,7 @@ struct dynamic::IterableProxy {
   }
 
  private:
-  const dynamic::ObjectImpl* o_;
+  object_type* o_;
 };
 
 inline dynamic::IterableProxy<dynamic::const_key_iterator> dynamic::keys()
@@ -341,6 +389,14 @@ inline dynamic::IterableProxy<dynamic::const_item_iterator> dynamic::items()
   return &(get<ObjectImpl>());
 }
 
+inline dynamic::IterableProxy<dynamic::value_iterator> dynamic::values() {
+  return &(get<ObjectImpl>());
+}
+
+inline dynamic::IterableProxy<dynamic::item_iterator> dynamic::items() {
+  return &(get<ObjectImpl>());
+}
+
 inline bool dynamic::isString() const {
   return get_nothrow<std::string>() != nullptr;
 }
@@ -527,6 +583,9 @@ inline std::size_t dynamic::count(dynamic const& key) const {
 inline dynamic::const_item_iterator dynamic::find(dynamic const& key) const {
   return get<ObjectImpl>().find(key);
 }
+inline dynamic::item_iterator dynamic::find(dynamic const& key) {
+  return get<ObjectImpl>().find(key);
+}
 
 template<class K, class V> inline void dynamic::insert(K&& key, V&& val) {
   auto& obj = get<ObjectImpl>();
@@ -574,7 +633,7 @@ inline std::size_t dynamic::erase(dynamic const& key) {
   return obj.erase(key);
 }
 
-inline dynamic::const_iterator dynamic::erase(const_iterator it) {
+inline dynamic::iterator dynamic::erase(const_iterator it) {
   auto& arr = get<Array>();
   // std::vector doesn't have an erase method that works on const iterators,
   // even though the standard says it should, so this hack converts to a
@@ -586,30 +645,31 @@ inline dynamic::const_key_iterator dynamic::erase(const_key_iterator it) {
   return const_key_iterator(get<ObjectImpl>().erase(it.base()));
 }
 
-inline dynamic::const_key_iterator dynamic::erase(const_key_iterator first,
-                                                  const_key_iterator last) {
+inline dynamic::const_key_iterator dynamic::erase(
+    const_key_iterator first,
+    const_key_iterator last) {
   return const_key_iterator(get<ObjectImpl>().erase(first.base(),
                                                     last.base()));
 }
 
-inline dynamic::const_value_iterator dynamic::erase(const_value_iterator it) {
-  return const_value_iterator(get<ObjectImpl>().erase(it.base()));
+inline dynamic::value_iterator dynamic::erase(const_value_iterator it) {
+  return value_iterator(get<ObjectImpl>().erase(it.base()));
 }
 
-inline dynamic::const_value_iterator dynamic::erase(const_value_iterator first,
-                                                    const_value_iterator last) {
-  return const_value_iterator(get<ObjectImpl>().erase(first.base(),
-                                                      last.base()));
+inline dynamic::value_iterator dynamic::erase(
+    const_value_iterator first,
+    const_value_iterator last) {
+  return value_iterator(get<ObjectImpl>().erase(first.base(), last.base()));
 }
 
-inline dynamic::const_item_iterator dynamic::erase(const_item_iterator it) {
-  return const_item_iterator(get<ObjectImpl>().erase(it.base()));
+inline dynamic::item_iterator dynamic::erase(const_item_iterator it) {
+  return item_iterator(get<ObjectImpl>().erase(it.base()));
 }
 
-inline dynamic::const_item_iterator dynamic::erase(const_item_iterator first,
-                                                   const_item_iterator last) {
-  return const_item_iterator(get<ObjectImpl>().erase(first.base(),
-                                                     last.base()));
+inline dynamic::item_iterator dynamic::erase(
+    const_item_iterator first,
+    const_item_iterator last) {
+  return item_iterator(get<ObjectImpl>().erase(first.base(), last.base()));
 }
 
 inline void dynamic::resize(std::size_t sz, dynamic const& c) {

diff --git a/folly/dynamic.cpp b/folly/dynamic.cpp
index c2fac276628b2d2842708c773d3e11e6d446a179..cc1ffdea211cf590e790e4d1c22a7a722643f8d7 100644 (file)
--- a/folly/dynamic.cpp
+++ b/folly/dynamic.cpp
@@ -254,8 +254,7 @@ std::size_t dynamic::size() const {
   throw TypeError("array/object", type());
 }
 
-dynamic::const_iterator
-dynamic::erase(const_iterator first, const_iterator last) {
+dynamic::iterator dynamic::erase(const_iterator first, const_iterator last) {
   auto& arr = get<Array>();
   return get<Array>().erase(
     arr.begin() + (first - arr.begin()),

diff --git a/folly/dynamic.h b/folly/dynamic.h
index 47a40d991e3e3d5ce52933d1f9cbfd3789919613..1c9ac0e0fea563eb7c526a7898f1479434a75e1e 100644 (file)
--- a/folly/dynamic.h
+++ b/folly/dynamic.h
@@ -99,12 +99,17 @@ struct dynamic : private boost::operators<dynamic> {
 private:
   typedef std::vector<dynamic> Array;
 public:
+  typedef Array::iterator iterator;
   typedef Array::const_iterator const_iterator;
   typedef dynamic value_type;
+
   struct const_key_iterator;
   struct const_value_iterator;
   struct const_item_iterator;
 
+  struct value_iterator;
+  struct item_iterator;
+
   /*
    * Creation routines for making dynamic objects and arrays.  Objects
    * are maps from key to value (so named due to json-related origins
@@ -320,6 +325,8 @@ public:
    */
   const_iterator begin()  const;
   const_iterator end()    const;
+  iterator begin();
+  iterator end();
 
 private:
   /*
@@ -335,6 +342,8 @@ public:
   IterableProxy<const_key_iterator> keys() const;
   IterableProxy<const_value_iterator> values() const;
   IterableProxy<const_item_iterator> items() const;
+  IterableProxy<value_iterator> values();
+  IterableProxy<item_iterator> items();
 
   /*
    * AssociativeContainer-style find interface for objects.  Throws if
@@ -344,6 +353,7 @@ public:
    * const_item_iterator pointing to the item.
    */
   const_item_iterator find(dynamic const&) const;
+  item_iterator find(dynamic const&);
 
   /*
    * If this is an object, returns whether it contains a field with
@@ -475,19 +485,17 @@ public:
    * removed, or end() if there are none.  (The iteration order does
    * not change.)
    */
-  const_iterator erase(const_iterator it);
-  const_iterator erase(const_iterator first, const_iterator last);
+  iterator erase(const_iterator it);
+  iterator erase(const_iterator first, const_iterator last);
 
   const_key_iterator erase(const_key_iterator it);
   const_key_iterator erase(const_key_iterator first, const_key_iterator last);
 
-  const_value_iterator erase(const_value_iterator it);
-  const_value_iterator erase(const_value_iterator first,
-                             const_value_iterator last);
+  value_iterator erase(const_value_iterator it);
+  value_iterator erase(const_value_iterator first, const_value_iterator last);
 
-  const_item_iterator erase(const_item_iterator it);
-  const_item_iterator erase(const_item_iterator first,
-                            const_item_iterator last);
+  item_iterator erase(const_item_iterator it);
+  item_iterator erase(const_item_iterator first, const_item_iterator last);
   /*
    * Append elements to an array.  If this is not an array, throws
    * TypeError.

diff --git a/folly/test/DynamicTest.cpp b/folly/test/DynamicTest.cpp
index b59c31f22534ae229b4ca479b418da787eaeb8d7..ff60856769cd190703628267066d3f9584e1757f 100644 (file)
--- a/folly/test/DynamicTest.cpp
+++ b/folly/test/DynamicTest.cpp
@@ -491,3 +491,135 @@ TEST(Dynamic, PrintNull) {
   ss << folly::dynamic(nullptr);
   EXPECT_EQ("null", ss.str());
 }
+
+TEST(Dynamic, WriteThroughArrayIterators) {
+  dynamic const cint(0);
+  dynamic d = dynamic::array(cint, cint, cint);
+  size_t size = d.size();
+
+  for (auto& val : d) {
+    EXPECT_EQ(val, cint);
+  }
+  EXPECT_EQ(d.size(), size);
+
+  dynamic ds(make_long_string());
+  for (auto& val : d) {
+    val = ds; // assign through reference
+  }
+
+  ds = "short string";
+  dynamic ds2(make_long_string());
+
+  for (auto& val : d) {
+    EXPECT_EQ(val, ds2);
+  }
+  EXPECT_EQ(d.size(), size);
+}
+
+TEST(Dynamic, MoveOutOfArrayIterators) {
+  dynamic ds(make_long_string());
+  dynamic d = dynamic::array(ds, ds, ds);
+  size_t size = d.size();
+
+  for (auto& val : d) {
+    EXPECT_EQ(val, ds);
+  }
+  EXPECT_EQ(d.size(), size);
+
+  for (auto& val : d) {
+    dynamic waste = std::move(val); // force moving out
+    EXPECT_EQ(waste, ds);
+  }
+
+  for (auto& val : d) {
+    EXPECT_NE(val, ds);
+  }
+  EXPECT_EQ(d.size(), size);
+}
+
+TEST(Dynamic, WriteThroughObjectIterators) {
+  dynamic const cint(0);
+  dynamic d = dynamic::object("key1", cint)("key2", cint);
+  size_t size = d.size();
+
+  for (auto& val : d.items()) {
+    EXPECT_EQ(val.second, cint);
+  }
+  EXPECT_EQ(d.size(), size);
+
+  dynamic ds(make_long_string());
+  for (auto& val : d.items()) {
+    val.second = ds; // assign through reference
+  }
+
+  ds = "short string";
+  dynamic ds2(make_long_string());
+  for (auto& val : d.items()) {
+    EXPECT_EQ(val.second, ds2);
+  }
+  EXPECT_EQ(d.size(), size);
+}
+
+TEST(Dynamic, MoveOutOfObjectIterators) {
+  dynamic ds(make_long_string());
+  dynamic d = dynamic::object("key1", ds)("key2", ds);
+  size_t size = d.size();
+
+  for (auto& val : d.items()) {
+    EXPECT_EQ(val.second, ds);
+  }
+  EXPECT_EQ(d.size(), size);
+
+  for (auto& val : d.items()) {
+    dynamic waste = std::move(val.second); // force moving out
+    EXPECT_EQ(waste, ds);
+  }
+
+  for (auto& val : d.items()) {
+    EXPECT_NE(val.second, ds);
+  }
+  EXPECT_EQ(d.size(), size);
+}
+
+TEST(Dynamic, ArrayIteratorInterop) {
+  dynamic d = dynamic::array(0, 1, 2);
+  dynamic const& cdref = d;
+
+  auto it = d.begin();
+  auto cit = cdref.begin();
+
+  EXPECT_EQ(it, cit);
+  EXPECT_EQ(cit, d.begin());
+  EXPECT_EQ(it, cdref.begin());
+
+  // Erase using non-const iterator
+  it = d.erase(it);
+  cit = cdref.begin();
+  EXPECT_EQ(*it, 1);
+  EXPECT_EQ(cit, it);
+
+  // Assign from non-const to const, preserve equality
+  decltype(cit) cit2 = it;
+  EXPECT_EQ(cit, cit2);
+}
+
+TEST(Dynamic, ObjectIteratorInterop) {
+  dynamic ds = make_long_string();
+  dynamic d = dynamic::object(0, ds)(1, ds)(2, ds);
+  dynamic const& cdref = d;
+
+  auto it = d.find(0);
+  auto cit = cdref.find(0);
+  EXPECT_NE(it, cdref.items().end());
+  EXPECT_NE(cit, cdref.items().end());
+  EXPECT_EQ(it, cit);
+
+  ++cit;
+  // Erase using non-const iterator
+  auto it2 = d.erase(it);
+  EXPECT_EQ(cit, it2);
+
+  // Assign from non-const to const, preserve equality
+  decltype(cit) cit2 = it2;
+  EXPECT_EQ(cit, cit2);
+}