diff --git a/core/base/iterator_factory.hpp b/core/base/iterator_factory.hpp index d8e9a2ac49e..e4133c2d2b1 100644 --- a/core/base/iterator_factory.hpp +++ b/core/base/iterator_factory.hpp @@ -34,355 +34,334 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #define GKO_CORE_BASE_ITERATOR_FACTORY_HPP_ +#include +#include +#include #include - - -#include +#include +#include namespace gko { namespace detail { +template +class zip_iterator; + + /** - * @internal - * @brief This class is used to sort two distinct arrays (`dominant_values` and - * `secondary_values`) with the same number of elements (it can be different - * types) according to the `dominant_values` in ascending order. + * A reference-like type pointing to a tuple of elements originating from a + * tuple of iterators. A few caveats related to its use: * - * Stores the pointers of two arrays, one storing the type `SecondaryType`, and - * one storing the type `ToSortType`. This class also provides an iterator - * class, which can be used for `std::sort`. Without a custom iterator, memory - * copies would be necessary to create, for example, one array of `std::pairs` - * to use `std::sort`, or a self written sort function. + * 1. It should almost never be stored as a reference, i.e. + * `auto& ref = *it` leads to a dangling reference, since the + * `zip_iterator_reference` returned by `*it` is a temporary. * - * Example of using this class to sort a list of people according to their age: - * ----------------------------------------------------------------- - * ```cpp - * std::vector age{50, 44, 43, 42}; - * std::vector person{"Karl", "Susanne", "Max", "Hannah"}; - * IteratorFactory factory{age.data(), person.data(), 4}; - * std::sort(factory.begin(), factory.end()); - * ``` - * Here, `person` now contains: `{"Hannah", "Max", "Susanne", "Karl"}` and - * `age` is now `{42, 43, 44, 50}`. Therefore, both arrays are now sorted - * according to the values in ascending order of `age`. + * 2. Any copy of the object is itself a reference to the same entry, i.e. + * `auto ref_copy = ref` means that assigning values to `ref_copy` also changes + * the data referenced by `ref` + * + * 3. If you want to copy the data, assign it to a variable of value_type: + * `tuple val = ref` or use the `copy` member function + * `auto val = ref.copy()` * - * @tparam ToSortType Type of the values which will be used for sorting. - * It must support `operator<`. - * @tparam SecondaryType Type of the values which will be moved synchronous - * to the array of type `ToSortType`. No comparisons - * with this type will be performed. + * @see zip_iterator + * @tparam Iterators the iterators that are zipped together */ -template -class IteratorFactory { - // All nested classes are hidden, so they can't be misused -private: - /** - * Helper struct, needed for the default construction and assignment inside - * `std::sort` through Reference. They are used as an intermediate data - * type for some of the swaps `std::sort` performs. - */ - struct element { - ToSortType dominant; - SecondaryType secondary; - - friend bool operator<(const element& left, const element& right) - { - return left.dominant < right.dominant; - } - }; - - /** - * This class is used as a reference to a sorting target, which abstracts - * the existence of two distinct arrays. - * It meets the requirements of `MoveAssignable`, `MoveConstructible` - * In all comparisons, only the values of `dominant_values_` matter, while - * the corresponding value of `secondary_values_` will always be copied / - * moved / swapped to the same place. - */ - class Reference { - public: - using array_index_type = int64; - - // An empty reference makes no sense, so is is disabled - Reference() = delete; - - ~Reference() {} - - Reference(IteratorFactory* parent, array_index_type array_index) - : parent_(parent), arr_index_(array_index) - {} - - // Since it must be `MoveConstructible` - Reference(Reference&& other) - : parent_(other.parent_), arr_index_(std::move(other.arr_index_)) - {} - - Reference(const Reference& other) - : parent_(other.parent_), arr_index_(other.arr_index_) - {} - - - Reference& operator=(element other) - { - dominant() = other.dominant; - secondary() = other.secondary; - return *this; - } - - Reference& operator=(const Reference& other) - { - dominant() = other.dominant(); - secondary() = other.secondary(); - return *this; - } - - // Since it must be `MoveAssignable` - Reference& operator=(Reference&& other) - { - // In C++11, it is legal for a nested class to access private - // members of the parent class. - parent_->dominant_values_[arr_index_] = - std::move(other.parent_->dominant_values_[other.arr_index_]); - parent_->secondary_values_[arr_index_] = - std::move(other.parent_->secondary_values_[other.arr_index_]); - return *this; - } - - // Conversion operator to `element` - operator element() const { return {dominant(), secondary()}; } - - friend void swap(Reference a, Reference b) - { - std::swap(a.dominant(), b.dominant()); - std::swap(a.secondary(), b.secondary()); - } - - friend bool operator<(const Reference& left, const Reference& right) - { - return left.dominant() < right.dominant(); - } - - friend bool operator<(const Reference& left, const element& right) - { - return left.dominant() < right.dominant; - } - - friend bool operator<(const element& left, const Reference& right) - { - return left.dominant < right.dominant(); - } - - ToSortType& dominant() { return parent_->dominant_values_[arr_index_]; } - - const ToSortType& dominant() const - { - return parent_->dominant_values_[arr_index_]; - } - - SecondaryType& secondary() - { - return parent_->secondary_values_[arr_index_]; - } - - const SecondaryType& secondary() const - { - return parent_->secondary_values_[arr_index_]; - } - - private: - IteratorFactory* parent_; - array_index_type arr_index_; - }; - - /** - * The iterator that can be used for `std::sort`. It meets the requirements - * of `LegacyRandomAccessIterator` and `ValueSwappable`. - * For performance reasons, it is expected that all iterators that are - * compared / used with each other have the same `parent`, so the check - * if they are the same can be omitted. - * This class uses a single variable to keep track of where the iterator - * points to both arrays. - */ - class Iterator { - public: - // Needed to count as a `LegacyRandomAccessIterator` - using difference_type = typename Reference::array_index_type; - using value_type = element; - using pointer = Reference; - using reference = Reference; - using iterator_category = std::random_access_iterator_tag; - - Iterator() = default; - - ~Iterator() {} - - Iterator(IteratorFactory* parent, difference_type array_index) - : parent_(parent), arr_index_(array_index) - {} - - Iterator(const Iterator& other) - : parent_(other.parent_), arr_index_(other.arr_index_) - {} - - Iterator& operator=(const Iterator& other) - { - arr_index_ = other.arr_index_; - return *this; - } - - // Operators needed for the std::sort requirement of - // `LegacyRandomAccessIterator` - Iterator& operator+=(difference_type i) - { - arr_index_ += i; - return *this; - } - - Iterator& operator-=(difference_type i) - { - arr_index_ -= i; - return *this; - } - - Iterator& operator++() // Prefix increment (++i) - { - ++arr_index_; - return *this; - } - - Iterator operator++(int) // Postfix increment (i++) - { - Iterator temp(*this); - ++arr_index_; - return temp; - } - - Iterator& operator--() // Prefix decrement (--i) - { - --arr_index_; - return *this; - } - - Iterator operator--(int) // Postfix decrement (i--) - { - Iterator temp(*this); - --arr_index_; - return temp; - } - - Iterator operator+(difference_type i) const - { - return {parent_, arr_index_ + i}; - } - - friend Iterator operator+(difference_type i, const Iterator& iter) - { - return {iter.parent_, iter.arr_index_ + i}; - } - - Iterator operator-(difference_type i) const - { - return {parent_, arr_index_ - i}; - } - - difference_type operator-(const Iterator& other) const - { - return arr_index_ - other.arr_index_; - } - - Reference operator*() const { return {parent_, arr_index_}; } - - Reference operator[](difference_type idx) const - { - return {parent_, arr_index_ + idx}; - } - - // Comparable operators - bool operator==(const Iterator& other) const - { - return arr_index_ == other.arr_index_; - } - - bool operator!=(const Iterator& other) const - { - return arr_index_ != other.arr_index_; - } - - bool operator<(const Iterator& other) const - { - return arr_index_ < other.arr_index_; - } - - bool operator<=(const Iterator& other) const - { - return arr_index_ <= other.arr_index_; - } - - bool operator>(const Iterator& other) const - { - return arr_index_ > other.arr_index_; - } - - bool operator>=(const Iterator& other) const - { - return arr_index_ >= other.arr_index_; - } - - private: - IteratorFactory* parent_{}; - difference_type arr_index_{}; - }; +template +class zip_iterator_reference + : public std::tuple< + typename std::iterator_traits::reference...> { + using ref_tuple_type = + std::tuple::reference...>; + using value_type = + std::tuple::value_type...>; + using index_sequence = std::index_sequence_for; + + friend class zip_iterator; + + template + value_type cast_impl(std::index_sequence) const + { + return {std::get(*this)...}; + } + + template + void assign_impl(std::index_sequence, const value_type& other) + { + (void)std::initializer_list{ + (std::get(*this) = std::get(other), 0)...}; + } + + zip_iterator_reference(Iterators... it) : ref_tuple_type{*it...} {} + +public: + operator value_type() const { return cast_impl(index_sequence{}); } + + zip_iterator_reference& operator=(const value_type& other) + { + assign_impl(index_sequence{}, other); + return *this; + } + + value_type copy() const { return *this; } +}; + + +/** + * A generic iterator adapter that combines multiple separate random access + * iterators for types a, b, c, ... into an iterator over tuples of type + * (a, b, c, ...). + * Dereferencing it returns a reference-like zip_iterator_reference object, + * similar to std::vector bit references. Accesses through that reference + * to the individual tuple elements get translated to the corresponding + * iterator's references. + * + * @note Two zip_iterators can only be compared if each individual pair of + * wrapped iterators has the same distance. Otherwise the behavior is + * undefined. This means that the only guaranteed safe way to use multiple + * zip_iterators is if they are all derived from the same iterator: + * ``` + * Iterator i, j; + * auto it1 = make_zip_iterator(i, j); + * auto it2 = make_zip_iterator(i, j + 1); + * auto it3 = make_zip_iterator(i + 1, j + 1); + * auto it4 = it1 + 1; + * it1 == it2; // undefined + * it1 == it3; // well-defined false + * it3 == it4; // well-defined true + * ``` + * This property is checked automatically in Debug builds and assumed in + * Release builds. + * + * @see zip_iterator_reference + * @tparam Iterators the iterators to zip together + */ +template +class zip_iterator { + static_assert(sizeof...(Iterators) > 0, "Can't build empty zip iterator"); public: - /** - * Allows creating an iterator, which makes it look like the data consists - * of `size` `std::pair`s, which are also - * sortable (according to the value of `dominant_values`) as long as - * `ToSortType` can be comparable with `operator<`. No additional data is - * allocated, all operations performed through the Iterator object will be - * done on the given arrays. The iterators given by this object (through - * `begin()` and `end()`) can be used with `std::sort()`. - * @param dominant_values Array of at least `size` values, which are the - * only values considered when comparing values (for example while sorting) - * @param secondary_values Array of at least `size` values, which will not - * be considered when comparing. However, they will be moved / copied to the - * same place as their corresponding value in `dominant_values` (with the - * same index). - * @param size Size of the arrays when constructiong the iterators. - * @note Both arrays must have at least `size` elements, otherwise, the - * behaviour is undefined. - */ - IteratorFactory(ToSortType* dominant_values, - SecondaryType* secondary_values, size_type size) - : dominant_values_(dominant_values), - secondary_values_(secondary_values), - size_(size) - {} - - /** - * Creates an iterator pointing to the beginning of both arrays - * @returns an iterator pointing to the beginning of both arrays - */ - Iterator begin() { return {this, 0}; } - - /** - * Creates an iterator pointing to the (excluding) end of both arrays - * @returns an iterator pointing to the (excluding) end of both arrays - */ - Iterator end() + using difference_type = std::ptrdiff_t; + using value_type = + std::tuple::value_type...>; + using pointer = value_type*; + using reference = zip_iterator_reference; + using iterator_category = std::random_access_iterator_tag; + using index_sequence = std::index_sequence_for; + + explicit zip_iterator() = default; + + explicit zip_iterator(Iterators... its) : iterators_{its...} {} + + zip_iterator& operator+=(difference_type i) + { + forall([i](auto& it) { it += i; }); + return *this; + } + + zip_iterator& operator-=(difference_type i) + { + forall([i](auto& it) { it -= i; }); + return *this; + } + + zip_iterator& operator++() + { + forall([](auto& it) { it++; }); + return *this; + } + + zip_iterator operator++(int) + { + auto tmp = *this; + ++(*this); + return tmp; + } + + zip_iterator& operator--() + { + forall([](auto& it) { it--; }); + return *this; + } + + zip_iterator operator--(int) + { + auto tmp = *this; + --(*this); + return tmp; + } + + zip_iterator operator+(difference_type i) const + { + auto tmp = *this; + tmp += i; + return tmp; + } + + friend zip_iterator operator+(difference_type i, const zip_iterator& iter) + { + return iter + i; + } + + zip_iterator operator-(difference_type i) const + { + auto tmp = *this; + tmp -= i; + return tmp; + } + + difference_type operator-(const zip_iterator& other) const + { + return forall_check_consistent( + other, [](const auto& a, const auto& b) { return a - b; }); + } + + reference operator*() const + { + return deref_impl(std::index_sequence_for{}); + } + + reference operator[](difference_type i) const { return *(*this + i); } + + bool operator==(const zip_iterator& other) const + { + return forall_check_consistent( + other, [](const auto& a, const auto& b) { return a == b; }); + } + + bool operator!=(const zip_iterator& other) const + { + return !(*this == other); + } + + bool operator<(const zip_iterator& other) const { - return {this, static_cast(size_)}; + return forall_check_consistent( + other, [](const auto& a, const auto& b) { return a < b; }); + } + + bool operator<=(const zip_iterator& other) const + { + return forall_check_consistent( + other, [](const auto& a, const auto& b) { return a <= b; }); + } + + bool operator>(const zip_iterator& other) const + { + return !(*this <= other); + } + + bool operator>=(const zip_iterator& other) const + { + return !(*this < other); } private: - ToSortType* dominant_values_; - SecondaryType* secondary_values_; - size_type size_; + template + reference deref_impl(std::index_sequence) const + { + return reference{std::get(iterators_)...}; + } + + template + void forall(Functor fn) + { + forall_impl(fn, index_sequence{}); + } + + template + void forall_impl(Functor fn, std::index_sequence) + { + (void)std::initializer_list{ + (fn(std::get(iterators_)), 0)...}; + } + + template + void forall_impl(const zip_iterator& other, Functor fn, + std::index_sequence) const + { + (void)std::initializer_list{ + (fn(std::get(iterators_), std::get(other.iterators_)), + 0)...}; + } + + template + auto forall_check_consistent(const zip_iterator& other, Functor fn) const + { + auto it = std::get<0>(iterators_); + auto other_it = std::get<0>(other.iterators_); + auto result = fn(it, other_it); + forall_impl( + other, [&](auto a, auto b) { assert(it - other_it == a - b); }, + index_sequence{}); + return result; + } + + std::tuple iterators_; }; +template +zip_iterator...> make_zip_iterator(Iterators&&... it) +{ + return zip_iterator...>{ + std::forward(it)...}; +} + + +/** + * Swap function for zip iterator references. It takes care of creating a + * non-reference temporary to avoid the problem of a normal std::swap(): + * ``` + * // a and b are reference-like objects pointing to different entries + * auto tmp = a; // tmp is a reference-like type, so this is not a copy! + * a = b; // copies value at b to a, which also modifies tmp + * b = tmp; // copies value at tmp (= a) to b + * // now both a and b point to the same value that was originally at b + * ``` + * It is modelled after the behavior of std::vector bit references. + * To swap in generic code, use the pattern `using std::swap; swap(a, b);` + * + * @tparam Iterators the iterator types inside the corresponding zip_iterator + */ +template +void swap(zip_iterator_reference a, + zip_iterator_reference b) +{ + auto tmp = a.copy(); + a = b; + b = tmp; +} + + +/** + * @copydoc swap(zip_iterator_reference, zip_iterator_reference) + */ +template +void swap(typename zip_iterator::value_type& a, + zip_iterator_reference b) +{ + auto tmp = a; + a = b; + b = tmp; +} + + +/** + * @copydoc swap(zip_iterator_reference, zip_iterator_reference) + */ +template +void swap(zip_iterator_reference a, + typename zip_iterator::value_type& b) +{ + auto tmp = a.copy(); + a = b; + b = tmp; +} + + } // namespace detail } // namespace gko diff --git a/core/test/base/iterator_factory.cpp b/core/test/base/iterator_factory.cpp index df50a723bda..c292fff1ff9 100644 --- a/core/test/base/iterator_factory.cpp +++ b/core/test/base/iterator_factory.cpp @@ -34,6 +34,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include +#include #include @@ -43,6 +45,24 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "core/test/utils.hpp" +namespace std { + + +// add a comparison for std::complex to allow comparisons without custom +// comparator +// HERE BE DRAGONS! This is technically UB, since we are adding things to +// namespace std, but since it is only inside a test, it should be fine :) +template +bool operator<(const std::complex& a, + const std::complex& b) +{ + return a.real() < b.real(); +} + + +} // namespace std + + namespace { @@ -62,13 +82,6 @@ class IteratorFactory : public ::testing::Test { 9., 10., 11., 12., 13., 14., 15.} {} - template - void check_vector_equal(const std::vector& v1, - const std::vector& v2) - { - ASSERT_TRUE(std::equal(v1.begin(), v1.end(), v2.begin())); - } - // Require that Iterator has a `value_type` specified template bool is_sorted_iterator(Iterator begin, Iterator end) @@ -104,11 +117,11 @@ TYPED_TEST(IteratorFactory, EmptyIterator) { using index_type = typename TestFixture::index_type; using value_type = typename TestFixture::value_type; - auto test_iter = gko::detail::IteratorFactory( - nullptr, nullptr, 0); - ASSERT_TRUE(test_iter.begin() == test_iter.end()); - ASSERT_NO_THROW(std::sort(test_iter.begin(), test_iter.end())); + auto test_iter = gko::detail::make_zip_iterator( + nullptr, nullptr); + + ASSERT_NO_THROW(std::sort(test_iter, test_iter)); } @@ -119,12 +132,11 @@ TYPED_TEST(IteratorFactory, SortingReversedWithIterator) std::vector vec1{this->reversed_index}; std::vector vec2{this->ordered_value}; - auto test_iter = gko::detail::IteratorFactory( - vec1.data(), vec2.data(), vec1.size()); - std::sort(test_iter.begin(), test_iter.end()); + auto test_iter = gko::detail::make_zip_iterator(vec1.data(), vec2.data()); + std::sort(test_iter, test_iter + vec1.size()); - this->check_vector_equal(vec1, this->ordered_index); - this->check_vector_equal(vec2, this->reversed_value); + ASSERT_EQ(vec1, this->ordered_index); + ASSERT_EQ(vec2, this->reversed_value); } @@ -135,12 +147,11 @@ TYPED_TEST(IteratorFactory, SortingAlreadySortedWithIterator) std::vector vec1{this->ordered_index}; std::vector vec2{this->ordered_value}; - auto test_iter = gko::detail::IteratorFactory( - vec1.data(), vec2.data(), vec1.size()); - std::sort(test_iter.begin(), test_iter.end()); + auto test_iter = gko::detail::make_zip_iterator(vec1.data(), vec2.data()); + std::sort(test_iter, test_iter + vec1.size()); - this->check_vector_equal(vec1, this->ordered_index); - this->check_vector_equal(vec2, this->ordered_value); + ASSERT_EQ(vec1, this->ordered_index); + ASSERT_EQ(vec2, this->ordered_value); } @@ -151,10 +162,9 @@ TYPED_TEST(IteratorFactory, IteratorReferenceOperatorSmaller) std::vector vec1{this->reversed_index}; std::vector vec2{this->ordered_value}; - auto test_iter = gko::detail::IteratorFactory( - vec1.data(), vec2.data(), vec1.size()); + auto test_iter = gko::detail::make_zip_iterator(vec1.data(), vec2.data()); bool is_sorted = - this->is_sorted_iterator(test_iter.begin(), test_iter.end()); + this->is_sorted_iterator(test_iter, test_iter + vec1.size()); ASSERT_FALSE(is_sorted); } @@ -167,10 +177,9 @@ TYPED_TEST(IteratorFactory, IteratorReferenceOperatorSmaller2) std::vector vec1{this->ordered_index}; std::vector vec2{this->ordered_value}; - auto test_iter = gko::detail::IteratorFactory( - vec1.data(), vec2.data(), vec1.size()); + auto test_iter = gko::detail::make_zip_iterator(vec1.data(), vec2.data()); bool is_sorted = - this->is_sorted_iterator(test_iter.begin(), test_iter.end()); + this->is_sorted_iterator(test_iter, test_iter + vec1.size()); ASSERT_TRUE(is_sorted); } @@ -183,10 +192,10 @@ TYPED_TEST(IteratorFactory, IncreasingIterator) std::vector vec1{this->reversed_index}; std::vector vec2{this->ordered_value}; - auto test_iter = gko::detail::IteratorFactory( - vec1.data(), vec2.data(), vec1.size()); - auto begin = test_iter.begin(); + auto test_iter = gko::detail::make_zip_iterator(vec1.data(), vec2.data()); + auto begin = test_iter; auto plus_2 = begin + 2; + auto plus_2_rev = 2 + begin; auto plus_minus_2 = plus_2 - 2; auto increment_pre_2 = begin; ++increment_pre_2; @@ -197,16 +206,86 @@ TYPED_TEST(IteratorFactory, IncreasingIterator) auto increment_pre_test = begin; auto increment_post_test = begin; + // check results for equality ASSERT_TRUE(begin == plus_minus_2); ASSERT_TRUE(plus_2 == increment_pre_2); + ASSERT_TRUE(plus_2_rev == increment_pre_2); ASSERT_TRUE(increment_pre_2 == increment_post_2); ASSERT_TRUE(begin == increment_post_test++); ASSERT_TRUE(begin + 1 == ++increment_pre_test); - ASSERT_TRUE((*plus_2).dominant() == vec1[2]); - ASSERT_TRUE((*plus_2).secondary() == vec2[2]); + ASSERT_TRUE(std::get<0>(*plus_2) == vec1[2]); + ASSERT_TRUE(std::get<1>(*plus_2) == vec2[2]); + // check other comparison operators and difference + std::vector> its{ + begin, + plus_2, + plus_2_rev, + plus_minus_2, + increment_pre_2, + increment_post_2, + increment_pre_test, + increment_post_test, + begin + 5, + begin + 9}; + std::sort(its.begin(), its.end()); + std::vector dists; + std::vector ref_dists{0, 1, 0, 1, 0, 0, 0, 3, 4}; + for (int i = 0; i < its.size() - 1; i++) { + SCOPED_TRACE(i); + dists.push_back(its[i + 1] - its[i]); + auto equal = dists.back() > 0; + ASSERT_EQ(its[i + 1] > its[i], equal); + ASSERT_EQ(its[i] < its[i + 1], equal); + ASSERT_EQ(its[i] != its[i + 1], equal); + ASSERT_EQ(its[i] == its[i + 1], !equal); + ASSERT_EQ(its[i] >= its[i + 1], !equal); + ASSERT_EQ(its[i + 1] <= its[i], !equal); + ASSERT_TRUE(its[i + 1] >= its[i]); + ASSERT_TRUE(its[i] <= its[i + 1]); + } + ASSERT_EQ(dists, ref_dists); } +#ifndef NDEBUG + + +bool check_assertion_exit_code(int exit_code) +{ +#ifdef _MSC_VER + // MSVC picks up the exit code incorrectly, + // so we can only check that it exits + return true; +#else + return exit_code != 0; +#endif +} + + +TYPED_TEST(IteratorFactory, IncompatibleIteratorDeathTest) +{ + using index_type = typename TestFixture::index_type; + using value_type = typename TestFixture::value_type; + auto it1 = gko::detail::make_zip_iterator(this->ordered_index.data(), + this->ordered_value.data()); + auto it2 = gko::detail::make_zip_iterator(this->ordered_index.data() + 1, + this->ordered_value.data()); + + // a set of operations that return inconsistent results for the two + // different iterators + EXPECT_EXIT(it2 - it1, check_assertion_exit_code, ""); + EXPECT_EXIT(it2 == it1, check_assertion_exit_code, ""); + EXPECT_EXIT(it2 != it1, check_assertion_exit_code, ""); + EXPECT_EXIT(it1 < it2, check_assertion_exit_code, ""); + EXPECT_EXIT(it2 <= it1, check_assertion_exit_code, ""); + EXPECT_EXIT(it2 > it1, check_assertion_exit_code, ""); + EXPECT_EXIT(it1 >= it2, check_assertion_exit_code, ""); +} + + +#endif + + TYPED_TEST(IteratorFactory, DecreasingIterator) { using index_type = typename TestFixture::index_type; @@ -214,9 +293,8 @@ TYPED_TEST(IteratorFactory, DecreasingIterator) std::vector vec1{this->reversed_index}; std::vector vec2{this->ordered_value}; - auto test_iter = gko::detail::IteratorFactory( - vec1.data(), vec2.data(), vec1.size()); - auto iter = test_iter.begin() + 5; + auto test_iter = gko::detail::make_zip_iterator(vec1.data(), vec2.data()); + auto iter = test_iter + 5; auto minus_2 = iter - 2; auto minus_plus_2 = minus_2 + 2; auto decrement_pre_2 = iter; @@ -233,8 +311,8 @@ TYPED_TEST(IteratorFactory, DecreasingIterator) ASSERT_TRUE(decrement_pre_2 == decrement_post_2); ASSERT_TRUE(iter == decrement_post_test--); ASSERT_TRUE(iter - 1 == --decrement_pre_test); - ASSERT_TRUE((*minus_2).dominant() == vec1[3]); - ASSERT_TRUE((*minus_2).secondary() == vec2[3]); + ASSERT_TRUE(std::get<0>(*minus_2) == vec1[3]); + ASSERT_TRUE(std::get<1>(*minus_2) == vec2[3]); } @@ -246,17 +324,16 @@ TYPED_TEST(IteratorFactory, CorrectDereferencing) std::vector vec2{this->ordered_value}; constexpr int element_to_test = 3; - auto test_iter = gko::detail::IteratorFactory( - vec1.data(), vec2.data(), vec1.size()); - auto begin = test_iter.begin(); + auto test_iter = gko::detail::make_zip_iterator(vec1.data(), vec2.data()); + auto begin = test_iter; using value_type = typename decltype(begin)::value_type; auto to_test_ref = *(begin + element_to_test); value_type to_test_pair = to_test_ref; // Testing implicit conversion - ASSERT_TRUE(to_test_pair.dominant == vec1[element_to_test]); - ASSERT_TRUE(to_test_pair.dominant == to_test_ref.dominant()); - ASSERT_TRUE(to_test_pair.secondary == vec2[element_to_test]); - ASSERT_TRUE(to_test_pair.secondary == to_test_ref.secondary()); + ASSERT_TRUE(std::get<0>(to_test_pair) == vec1[element_to_test]); + ASSERT_TRUE(std::get<0>(to_test_pair) == std::get<0>(to_test_ref)); + ASSERT_TRUE(std::get<1>(to_test_pair) == vec2[element_to_test]); + ASSERT_TRUE(std::get<1>(to_test_pair) == std::get<1>(to_test_ref)); } @@ -267,10 +344,9 @@ TYPED_TEST(IteratorFactory, CorrectSwapping) std::vector vec1{this->reversed_index}; std::vector vec2{this->ordered_value}; - auto test_iter = gko::detail::IteratorFactory( - vec1.data(), vec2.data(), vec1.size()); - auto first_el_reference = *test_iter.begin(); - auto second_el_reference = *(test_iter.begin() + 1); + auto test_iter = gko::detail::make_zip_iterator(vec1.data(), vec2.data()); + auto first_el_reference = *test_iter; + auto second_el_reference = *(test_iter + 1); swap(first_el_reference, second_el_reference); ASSERT_TRUE(vec1[0] == this->reversed_index[1]); @@ -292,11 +368,10 @@ TYPED_TEST(IteratorFactory, CorrectHandWrittenSwapping) std::vector vec1{this->reversed_index}; std::vector vec2{this->ordered_value}; - auto test_iter = gko::detail::IteratorFactory( - vec1.data(), vec2.data(), vec1.size()); - auto first_el_reference = *test_iter.begin(); - auto second_el_reference = *(test_iter.begin() + 1); - auto temp = static_cast( + auto test_iter = gko::detail::make_zip_iterator(vec1.data(), vec2.data()); + auto first_el_reference = *test_iter; + auto second_el_reference = *(test_iter + 1); + auto temp = static_cast( first_el_reference); first_el_reference = second_el_reference; second_el_reference = temp; diff --git a/core/test/utils/unsort_matrix.hpp b/core/test/utils/unsort_matrix.hpp index 21464ead00e..df5ed109b48 100644 --- a/core/test/utils/unsort_matrix.hpp +++ b/core/test/utils/unsort_matrix.hpp @@ -81,9 +81,8 @@ void unsort_matrix(matrix::Csr* mtx, for (index_type row = 0; row < size[0]; ++row) { auto start = row_ptrs[row]; auto end = row_ptrs[row + 1]; - auto sort_wrapper = gko::detail::IteratorFactory( - cols + start, vals + start, end - start); - std::shuffle(sort_wrapper.begin(), sort_wrapper.end(), engine); + auto it = gko::detail::make_zip_iterator(cols + start, vals + start); + std::shuffle(it, it + (end - start), engine); } } diff --git a/omp/matrix/csr_kernels.cpp b/omp/matrix/csr_kernels.cpp index 5ba21041ae4..484f6966944 100644 --- a/omp/matrix/csr_kernels.cpp +++ b/omp/matrix/csr_kernels.cpp @@ -903,9 +903,11 @@ void sort_by_column_index(std::shared_ptr exec, for (size_type i = 0; i < number_rows; ++i) { auto start_row_idx = row_ptrs[i]; auto row_nnz = row_ptrs[i + 1] - start_row_idx; - auto helper = detail::IteratorFactory( - col_idxs + start_row_idx, values + start_row_idx, row_nnz); - std::sort(helper.begin(), helper.end()); + auto it = detail::make_zip_iterator(col_idxs + start_row_idx, + values + start_row_idx); + std::sort(it, it + row_nnz, [](auto t1, auto t2) { + return std::get<0>(t1) < std::get<0>(t2); + }); } } diff --git a/omp/matrix/fbcsr_kernels.cpp b/omp/matrix/fbcsr_kernels.cpp index 40846e366bd..ce647f79935 100644 --- a/omp/matrix/fbcsr_kernels.cpp +++ b/omp/matrix/fbcsr_kernels.cpp @@ -391,9 +391,10 @@ void sort_by_column_index_impl( std::vector col_permute(nbnz_brow); std::iota(col_permute.begin(), col_permute.end(), 0); - auto helper = detail::IteratorFactory( - brow_col_idxs, col_permute.data(), nbnz_brow); - std::sort(helper.begin(), helper.end()); + auto it = detail::make_zip_iterator(brow_col_idxs, col_permute.data()); + std::sort(it, it + nbnz_brow, [](auto a, auto b) { + return std::get<0>(a) < std::get<0>(b); + }); std::vector oldvalues(nbnz_brow * bs2); std::copy(brow_vals, brow_vals + nbnz_brow * bs2, oldvalues.begin()); diff --git a/omp/matrix/sparsity_csr_kernels.cpp b/omp/matrix/sparsity_csr_kernels.cpp index 172c4201760..3ed0bc507ce 100644 --- a/omp/matrix/sparsity_csr_kernels.cpp +++ b/omp/matrix/sparsity_csr_kernels.cpp @@ -46,7 +46,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include -#include "core/base/iterator_factory.hpp" #include "core/components/fill_array_kernels.hpp" #include "core/components/prefix_sum_kernels.hpp" diff --git a/reference/matrix/csr_kernels.cpp b/reference/matrix/csr_kernels.cpp index 92718652f90..ddc67160bae 100644 --- a/reference/matrix/csr_kernels.cpp +++ b/reference/matrix/csr_kernels.cpp @@ -874,9 +874,11 @@ void sort_by_column_index(std::shared_ptr exec, for (size_type i = 0; i < number_rows; ++i) { auto start_row_idx = row_ptrs[i]; auto row_nnz = row_ptrs[i + 1] - start_row_idx; - auto helper = detail::IteratorFactory( - col_idxs + start_row_idx, values + start_row_idx, row_nnz); - std::sort(helper.begin(), helper.end()); + auto it = detail::make_zip_iterator(col_idxs + start_row_idx, + values + start_row_idx); + std::sort(it, it + row_nnz, [](auto t1, auto t2) { + return std::get<0>(t1) < std::get<0>(t2); + }); } } diff --git a/reference/matrix/fbcsr_kernels.cpp b/reference/matrix/fbcsr_kernels.cpp index 8f20fe9a403..9b093a97b68 100644 --- a/reference/matrix/fbcsr_kernels.cpp +++ b/reference/matrix/fbcsr_kernels.cpp @@ -445,9 +445,10 @@ void sort_by_column_index_impl( std::vector col_permute(nbnz_brow); std::iota(col_permute.begin(), col_permute.end(), 0); - auto helper = detail::IteratorFactory( - brow_col_idxs, col_permute.data(), nbnz_brow); - std::sort(helper.begin(), helper.end()); + auto it = detail::make_zip_iterator(brow_col_idxs, col_permute.data()); + std::sort(it, it + nbnz_brow, [](auto a, auto b) { + return std::get<0>(a) < std::get<0>(b); + }); std::vector oldvalues(nbnz_brow * bs2); std::copy(brow_vals, brow_vals + nbnz_brow * bs2, oldvalues.begin()); diff --git a/reference/matrix/sparsity_csr_kernels.cpp b/reference/matrix/sparsity_csr_kernels.cpp index 53e3a8e485e..71e63f18554 100644 --- a/reference/matrix/sparsity_csr_kernels.cpp +++ b/reference/matrix/sparsity_csr_kernels.cpp @@ -43,7 +43,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include -#include "core/base/iterator_factory.hpp" #include "core/components/fill_array_kernels.hpp" #include "core/components/prefix_sum_kernels.hpp"