From f223c715ad640973448bfa9496b28531b22936e1 Mon Sep 17 00:00:00 2001 From: James Donald Date: Thu, 18 Jul 2024 05:08:59 -0700 Subject: [PATCH] Revert D59811691: customization for folly::index_iterator Differential Revision: D59811691 Original commit changeset: 7da2db4fcf30 Original Phabricator Diff: D59811691 fbshipit-source-id: d6b15e2232ba10a5fcafc4d15d5638c26e732612 --- folly/container/BUCK | 1 - folly/container/Iterator.h | 46 ++------------ folly/container/test/IteratorTest.cpp | 87 --------------------------- 3 files changed, 4 insertions(+), 130 deletions(-) diff --git a/folly/container/BUCK b/folly/container/BUCK index 07f07886304..b0182ab8b50 100644 --- a/folly/container/BUCK +++ b/folly/container/BUCK @@ -41,7 +41,6 @@ cpp_library( ":access", "//folly:traits", "//folly:utility", - "//folly/functional:invoke", "//folly/lang:rvalue_reference_wrapper", ], ) diff --git a/folly/container/Iterator.h b/folly/container/Iterator.h index 68e1baacf9a..22e66c7b032 100644 --- a/folly/container/Iterator.h +++ b/folly/container/Iterator.h @@ -26,7 +26,6 @@ #include #include #include -#include #include namespace folly { @@ -598,24 +597,8 @@ struct arrow_proxy { explicit arrow_proxy(Ref* ref) : res(*ref) {} }; -struct index_iterator_access_at { - template - constexpr decltype(auto) operator()(Container& container, Index index) const { - if constexpr (folly::is_tag_invocable_v< - index_iterator_access_at, - Container&, - Index>) { - return folly::tag_invoke(*this, container, index); - } else { - return container[index]; - } - } -}; - } // namespace detail -FOLLY_DEFINE_CPO(detail::index_iterator_access_at, index_iterator_access_at) - /** * index_iterator * @@ -642,20 +625,8 @@ FOLLY_DEFINE_CPO(detail::index_iterator_access_at, index_iterator_access_at) * Note that `some_ref_type` can be any proxy reference, as long as the * algorithms support that (for example from range-v3). * - * NOTE: if `operator[]` doesn't work for you for some reason - * you can specify: - * - * ``` - * friend some_ref_type tag_invoke( - * folly::cpo_t, - * Container& c, - * std::size_t index); - * - * friend some_cref_type tag_invoke( - * folly::cpo_t, - * const Container& c, - * std::size_t index); - * ``` + * NOTE: there is no way to override `operator[]`, if that's needed + * we recommend to wrap your data in a struct with `operator[]`. **/ template @@ -666,12 +637,6 @@ class index_iterator { template using get_difference_type_t = typename std::remove_cv_t::difference_type; - template - constexpr static decltype(auto) get_reference_by_index( - Container& container, IndexType index) { - return index_iterator_access_at(container, index); - } - public: // index iterator specific types @@ -682,8 +647,7 @@ class index_iterator { using value_type = typename std::remove_const_t::value_type; using iterator_category = std::random_access_iterator_tag; - using reference = decltype(get_reference_by_index( - FOLLY_DECLVAL(container_type&), size_type{})); + using reference = decltype(FOLLY_DECLVAL(container_type&)[size_type{}]); using difference_type = detected_or_t; @@ -719,9 +683,7 @@ class index_iterator { // access --- - constexpr reference operator*() const { - return get_reference_by_index(*container_, index_); - } + constexpr reference operator*() const { return (*container_)[index_]; } pointer operator->() const { // It's equivalent to pointer{&**this} but compiler stops diff --git a/folly/container/test/IteratorTest.cpp b/folly/container/test/IteratorTest.cpp index 2e65fc94304..280c7fd097a 100644 --- a/folly/container/test/IteratorTest.cpp +++ b/folly/container/test/IteratorTest.cpp @@ -681,42 +681,6 @@ struct has_both_size_and_difference_type : just_operator { using difference_type = std::int32_t; }; -struct no_operator { - using value_type = int; - - friend ref_type tag_invoke( - folly::cpo_t, no_operator&, std::size_t) { - return {}; - } - - friend cref_type tag_invoke( - folly::cpo_t, const no_operator&, std::size_t) { - return {}; - } - - [[maybe_unused]] int operator[](std::size_t); - [[maybe_unused]] int operator[](std::size_t) const; -}; - -struct no_operator_private { - using value_type = int; - - private: - friend ref_type tag_invoke( - folly::cpo_t, - no_operator_private&, - std::size_t) { - return {}; - } - - friend cref_type tag_invoke( - folly::cpo_t, - const no_operator_private&, - std::size_t) { - return {}; - } -}; - template using all_types = std::tuple< typename std::iterator_traits>::value_type, @@ -755,18 +719,6 @@ TEST(IndexIterator, Types) { is_same_test( all_types{}, std::tuple{}); - is_same_test( - all_types{}, - std::tuple{}); - is_same_test( - all_types{}, - std::tuple{}); - is_same_test( - all_types{}, - std::tuple{}); - is_same_test( - all_types{}, - std::tuple{}); } } // namespace index_iterator_type_tests @@ -880,31 +832,6 @@ struct IndexedVector { const_iterator cend() const { return {*this, v_.size()}; } }; -struct CustomMapWithIndexedIterator { - public: - using size_type = std::uint32_t; - using value_type = std::pair; - - public: - using iterator = folly::index_iterator; - using const_iterator = - folly::index_iterator; - - iterator begin() { return {*this, 0}; } - const_iterator cbegin() const { return {*this, 0}; } - - std::vector keys; - std::vector values; - - private: - friend std::pair tag_invoke( - folly::cpo_t, - const CustomMapWithIndexedIterator& self, - size_type idx) { - return {self.keys[idx], self.values[idx]}; - } -}; - } // namespace TEST(IndexIterator, UseProxyReferences) { @@ -973,17 +900,3 @@ TEST(IndexIterator, OperatorArrowForNonProxies) { ASSERT_EQ(expected, v); } - -TEST(IndexIterator, NoOperatorIntegration) { - CustomMapWithIndexedIterator m{.keys{1, 2, 3}, .values{4, 5, 6}}; - - auto it = m.begin(); - ASSERT_EQ((std::pair{1, 4}), it[0]); - ASSERT_EQ((std::pair{2, 5}), it[1]); - ASSERT_EQ((std::pair{3, 6}), it[2]); - - auto cit = m.cbegin(); - ASSERT_EQ((std::pair{1, 4}), cit[0]); - ASSERT_EQ((std::pair{2, 5}), cit[1]); - ASSERT_EQ((std::pair{3, 6}), cit[2]); -}