From 73182f6ef64011e8b2493b171d0ae448c89a6178 Mon Sep 17 00:00:00 2001 From: Andrew Krieger Date: Wed, 8 Feb 2017 16:03:39 -0800 Subject: [PATCH] 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 --- folly/dynamic-inl.h | 96 ++++++++++++++++++++++----- folly/dynamic.cpp | 3 +- folly/dynamic.h | 24 ++++--- folly/test/DynamicTest.cpp | 132 +++++++++++++++++++++++++++++++++++++ 4 files changed, 227 insertions(+), 28 deletions(-) diff --git a/folly/dynamic-inl.h b/folly/dynamic-inl.h index 41352ad0..dbe36b5d 100644 --- 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 { /* 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().end(); } +inline dynamic::iterator dynamic::begin() { + return get().begin(); +} +inline dynamic::iterator dynamic::end() { + return get().end(); +} + template 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::keys() @@ -341,6 +389,14 @@ inline dynamic::IterableProxy dynamic::items() return &(get()); } +inline dynamic::IterableProxy dynamic::values() { + return &(get()); +} + +inline dynamic::IterableProxy dynamic::items() { + return &(get()); +} + inline bool dynamic::isString() const { return get_nothrow() != 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().find(key); } +inline dynamic::item_iterator dynamic::find(dynamic const& key) { + return get().find(key); +} template inline void dynamic::insert(K&& key, V&& val) { auto& obj = get(); @@ -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(); // 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().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().erase(first.base(), last.base())); } -inline dynamic::const_value_iterator dynamic::erase(const_value_iterator it) { - return const_value_iterator(get().erase(it.base())); +inline dynamic::value_iterator dynamic::erase(const_value_iterator it) { + return value_iterator(get().erase(it.base())); } -inline dynamic::const_value_iterator dynamic::erase(const_value_iterator first, - const_value_iterator last) { - return const_value_iterator(get().erase(first.base(), - last.base())); +inline dynamic::value_iterator dynamic::erase( + const_value_iterator first, + const_value_iterator last) { + return value_iterator(get().erase(first.base(), last.base())); } -inline dynamic::const_item_iterator dynamic::erase(const_item_iterator it) { - return const_item_iterator(get().erase(it.base())); +inline dynamic::item_iterator dynamic::erase(const_item_iterator it) { + return item_iterator(get().erase(it.base())); } -inline dynamic::const_item_iterator dynamic::erase(const_item_iterator first, - const_item_iterator last) { - return const_item_iterator(get().erase(first.base(), - last.base())); +inline dynamic::item_iterator dynamic::erase( + const_item_iterator first, + const_item_iterator last) { + return item_iterator(get().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 c2fac276..cc1ffdea 100644 --- 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(); return get().erase( arr.begin() + (first - arr.begin()), diff --git a/folly/dynamic.h b/folly/dynamic.h index 47a40d99..1c9ac0e0 100644 --- a/folly/dynamic.h +++ b/folly/dynamic.h @@ -99,12 +99,17 @@ struct dynamic : private boost::operators { private: typedef std::vector 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 keys() const; IterableProxy values() const; IterableProxy items() const; + IterableProxy values(); + IterableProxy 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 b59c31f2..ff608567 100644 --- 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); +} -- 2.34.1