From 7447950c6c3c0684c8bbb67a0c25526a8c7ad04b Mon Sep 17 00:00:00 2001 From: hoxnox Date: Wed, 11 May 2016 15:09:03 +0300 Subject: [PATCH 01/16] ENH: shuffled released --- README.md | 20 ++++ shuffled.hpp | 185 ++++++++++++++++++++++++++++++++++ test/SConstruct | 1 + test/test_shuffled.cpp | 224 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 430 insertions(+) create mode 100644 shuffled.hpp create mode 100644 test/test_shuffled.cpp diff --git a/README.md b/README.md index b293c690..474b2e99 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,7 @@ evaluation wherever possible. [accumulate](#accumulate)
[compress](#compress)
[sorted](#sorted)
+[shuffled](#shuffled)
[chain](#chain)
[chain.from\_iterable](#chainfrom_iterable)
[reversed](#reversed)
@@ -525,6 +526,25 @@ an `operator*() const` member. The below outputs `0 1 2 3 4`. +```c++ +unordered_set nums{4, 0, 2, 1, 3}; +for (auto&& i : sorted(nums)) { + cout << i << ' '; +} +``` + +shuffled +-------- +*Additional Requirements*: Input must have a ForwardIterator. + +Allows iteration over a sequence in shuffled order. `shuffled` does +**not** produce a new sequence, copy elements, or modify the original +sequence. It only provides a way to iterate over existing elements. +`shuffled` also takes an optional second argument - randomization seed. +If not provided, defaults to 1. + +The below outputs `0 2 4 3 1`. + ```c++ unordered_set nums{4, 0, 2, 1, 3}; for (auto&& i : sorted(nums)) { diff --git a/shuffled.hpp b/shuffled.hpp new file mode 100644 index 00000000..4f7073e2 --- /dev/null +++ b/shuffled.hpp @@ -0,0 +1,185 @@ +#ifndef ITER_SHUFFLED_HPP_ +#define ITER_SHUFFLED_HPP_ + +#include "internal/iterbase.hpp" + +/* Suppose the size of the container is N. We can find the power of two, + * that is greater or equal to N - K. Numbers from 1 to K can be easy + * shuffled with help of Linear Feedback Shift Register (LFSR) using + * special prime poly. Access to every element of the shuffled is + * implemented through advance of the first (begin) iterator of the + * container, so random access iterator is desirable. In the constructor + * we have to calculate the total size of the container, so + * dumb_distance will be used.*/ + +namespace iter { + namespace impl { + template + class ShuffledView; + // linear feedback shift register + namespace lfsr { + const uint16_t PRIME_POLY[64] = { + 1, 3, 3, 3, 5, 3, 3, 29, 17, 9, 5, 83, 27, 43, 3, 45, 9, 129, 39, + 9, 5, 3, 33, 27, 9, 71, 39, 9, 5, 83, 9, 197, 8193, 231, 5, 119, + 83, 99, 17, 57, 9, 153, 89, 101, 27, 449, 33, 183, 113, 29, 75, + 9, 71, 125, 71, 149, 129, 99, 123, 3, 39, 105, 3, 27 + }; // prime poly for full-cycle shift registers + uint16_t get_approx(uint64_t val); + uint64_t shift(uint64_t reg, uint8_t reg_size); + } + } + + template + impl::ShuffledView shuffled(Container&&, int seed = 1); +} + +// power of 2 approximation +uint16_t iter::impl::lfsr::get_approx(uint64_t val) { + if (val == 0) + return 0; + uint16_t pow2_approx = 0; + if (val > 9223372036854775808UL) { + pow2_approx = 63; + } + else { + while (pow2_approx < 64) { + if (val >= (1UL << (pow2_approx + 1))) + ++pow2_approx; + else + break; + } + } + return pow2_approx + 1; +} + +uint64_t iter::impl::lfsr::shift(uint64_t reg, uint8_t reg_size) { + if (reg & 1) + reg = ((reg ^ PRIME_POLY[reg_size]) >> 1) | (1 << reg_size); + else + reg >>= 1; + return reg; +} + +template +class iter::impl::ShuffledView { + private: + + Distance size; + uint8_t size_approx; + iterator_type in_begin; + uint64_t seed; + + template + friend ShuffledView iter::shuffled(C&&, int); + + + public: + using IterDeref = typename std::remove_reference>; + ShuffledView(ShuffledView&&) : size(0) {}; + ShuffledView(Container&& container, int seed) + : size(dumb_size(container)), size_approx(lfsr::get_approx(size)), + in_begin(std::begin(container)), seed(seed) { + if (size > 0) + { + uint64_t mask = 0; + std::uninitialized_fill((char*)&mask, (char*)&mask + size_approx, 0xFF); + this->seed = seed & mask; + this->seed = lfsr::shift(this->seed, size_approx); + while(this->seed >= size) + this->seed = lfsr::shift(this->seed, size_approx); + } + } + + class Iterator + : public std::iterator { + private: + friend class ShuffledView; + ShuffledView* owner; + uint64_t state; + iterator_type copy; // referenced by operator* value + + public: + Iterator() : owner(nullptr), state(0) {} + Iterator& operator=(const Iterator& other) { + owner = other.owner; + state = other.state; + return *this; + }; + + Iterator& operator++() { + if (operator==(owner->end())) + return *this; + state = lfsr::shift(state, owner->size_approx); + while(state > owner->size) + state = lfsr::shift(state, owner->size_approx); + if (state == owner->seed) + operator=(owner->end()); + return *this; + } + + Iterator operator++(int) { + auto ret = *this; + ++*this; + return ret; + } + + bool operator==(const Iterator& other) const { + return owner == other.owner && state == other.state; + } + + bool operator!=(const Iterator& other) const { + return !operator==(other); + } + + auto operator*() -> decltype(*copy) { + copy = owner->in_begin; + dumb_advance(copy, static_cast(state-1)); + return *copy; + } + + ArrowProxy operator->() { + return {**this}; + } + }; + + Iterator begin() { + Iterator begin; + begin.owner = this; + begin.state = (size == 0 ? 0 : seed); + return begin; + } + + Iterator end() { + Iterator end; + end.owner = this; + end.state = 0; + return end; + } + + /* @brief restore iteration through a vector in shuffled order from the + * index of the last seen element in non shuffled container + * + * @example: + * v = {"apple", "banana", "orange", "peach"}; // original vector + * s = shuffled(v); // {"orange", "peach", "banana", "apple"} - shuffled vector + * + * We iterated through s. Last seen element was "banana". In the + * original vector "banana" had index 1. So to restore shuffling wee + * need to do s.restore(1); Operation is zero cost - no iterators + * advance is needed.*/ + Iterator restore(uint64_t state) { + Iterator rs; + rs.owner = this; + rs.state = (state >= size ? seed : state + 1); + return rs; + } +}; + +template +iter::impl::ShuffledView iter::shuffled( + Container&& container, int seed = 1) { + return {std::forward(container), seed}; +} + +#endif + diff --git a/test/SConstruct b/test/SConstruct index d9513f5f..2c7fae77 100644 --- a/test/SConstruct +++ b/test/SConstruct @@ -36,6 +36,7 @@ progs = Split( slice sliding_window sorted + shuffled takewhile unique_everseen unique_justseen diff --git a/test/test_shuffled.cpp b/test/test_shuffled.cpp new file mode 100644 index 00000000..e958289a --- /dev/null +++ b/test/test_shuffled.cpp @@ -0,0 +1,224 @@ +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "helpers.hpp" +#include "catch.hpp" + +using iter::shuffled; + +using Vec = const std::vector; + +TEST_CASE("shuffled: iterates through a vector in shuffled order", "[shuffled]") { + Vec ns = {4, 0, 5, 1, 6, 7, 9, 3, 2, 8}; + auto s = shuffled(ns); + Vec v(std::begin(s), std::end(s)); + Vec vc = {2, 7, 5, 9, 8, 6, 3, 1, 0, 4}; + REQUIRE(v == vc); +} + +TEST_CASE("shuffled: iterates through a list of strings in shuffled order", + "[shuffled]") { + std::list ns = {"apple", "banana", "orange", "peach"}; + auto s = shuffled(ns); + std::list v(std::begin(s), std::end(s)); + std::list vc = {"orange", "peach", "banana", "apple"}; + REQUIRE(v == vc); +} + +TEST_CASE("shuffled: restore iteration through a vector in shuffled order " + "from the index of the last seen element in non shuffled container", + "[shuffled]") { + //index: 0 1 2 3 4 5 6 7 8 9 + Vec ns = {4, 0, 5, 1, 6, 7, 9, 3, 2, 8}; + auto s = shuffled(ns); + + // overflow - same as default + Vec v10(s.restore(10), std::end(s)); + Vec vc10 = {2, 7, 5, 9, 8, 6, 3, 1, 0, 4}; + REQUIRE(v10 == vc10); + + Vec v1(s.restore(5), std::end(s)); + Vec vc1 = {7, 5, 9, 8, 6, 3, 1, 0, 4}; + REQUIRE(v1 == vc1); + + Vec v2(s.restore(2), std::end(s)); + Vec vc2 = {5, 9, 8, 6, 3, 1, 0, 4}; + REQUIRE(v2 == vc2); + + Vec v3(s.restore(6), std::end(s)); + Vec vc3 = {9, 8, 6, 3, 1, 0, 4}; + REQUIRE(v3 == vc3); + + Vec v4(s.restore(9), std::end(s)); + Vec vc4 = {8, 6, 3, 1, 0, 4}; + REQUIRE(v4 == vc4); + + Vec v5(s.restore(4), std::end(s)); + Vec vc5 = {6, 3, 1, 0, 4}; + REQUIRE(v5 == vc5); + + Vec v6(s.restore(7), std::end(s)); + Vec vc6 = {3, 1, 0, 4}; + REQUIRE(v6 == vc6); + + Vec v7(s.restore(3), std::end(s)); + Vec vc7 = {1, 0, 4}; + REQUIRE(v7 == vc7); + + Vec v8(s.restore(1), std::end(s)); + Vec vc8 = {0, 4}; + REQUIRE(v8 == vc8); + + Vec v9(s.restore(0), std::end(s)); + Vec vc9 = {4}; + REQUIRE(v9 == vc9); +} + +TEST_CASE("shuffled: restore iteration through a vector in shuffled order " + "from the index of the last seen element in non shuffled " + "container of strings", "[shuffled]") { + std::list ns = {"apple", "banana", "orange", "peach"}; + auto s = shuffled(ns); + std::list v(s.restore(1), std::end(s)); + std::list vc = {"banana", "apple"}; + REQUIRE(v == vc); +} + +TEST_CASE("shuffled: iterates through a vector in shuffled order with non" + " standard seed", "[shuffled]") { + Vec ns = {4, 0, 5, 1, 6, 7, 9, 3, 2, 8}; + auto s = shuffled(ns, 1234); + Vec v(std::begin(s), std::end(s)); + Vec vc = {9, 8, 6, 3, 1, 0, 4, 2, 7, 5}; + REQUIRE(v == vc); +} + +TEST_CASE("shuffled: can modify elements through shuffled", "[shuffled]") { + std::vector ns(3, 9); + for (auto&& n : shuffled(ns)) { + n = -1; + } + Vec vc(3, -1); + REQUIRE(ns == vc); +} + +TEST_CASE("shuffled: can iterate over unordered container", "[shuffled]") { + std::unordered_set ns = {1, 3, 2, 0, 4}; + auto s = shuffled(ns); + + Vec v(std::begin(s), std::end(s)); + Vec vc = {3, 2, 1, 4, 0}; + REQUIRE(v == vc); +} + +TEST_CASE("shuffled: empty when iterable is empty", "[shuffled]") { + Vec ns{}; + auto s = shuffled(ns); + REQUIRE(std::begin(s) == std::end(s)); +} + +namespace { + template + class BasicIterableWithConstDeref { + private: + T* data; + std::size_t size; + bool was_moved_from_ = false; + + public: + BasicIterableWithConstDeref(std::initializer_list il) + : data{new T[il.size()]}, size{il.size()} { + // would like to use enumerate, can't because it's for unit + // testing enumerate + std::size_t i = 0; + for (auto&& e : il) { + data[i] = e; + ++i; + } + } + + BasicIterableWithConstDeref& operator=( + BasicIterableWithConstDeref&&) = delete; + BasicIterableWithConstDeref& operator=( + const BasicIterableWithConstDeref&) = delete; + BasicIterableWithConstDeref(const BasicIterableWithConstDeref&) = delete; + + BasicIterableWithConstDeref(BasicIterableWithConstDeref&& other) + : data{other.data}, size{other.size} { + other.data = nullptr; + other.was_moved_from_ = true; + } + + bool was_moved_from() const { + return this->was_moved_from_; + } + + ~BasicIterableWithConstDeref() { + delete[] this->data; + } + + class Iterator + : public std::iterator { + private: + T* p; + + public: + Iterator(T* b) : p{b} {} + bool operator!=(const Iterator& other) const { + return this->p != other.p; + } + + Iterator& operator++() { + ++this->p; + return *this; + } + + T& operator*() { + return *this->p; + } + + const T& operator*() const { + return *this->p; + } + }; + + Iterator begin() { + return {this->data}; + } + + Iterator end() { + return {this->data + this->size}; + } + }; +} + +TEST_CASE("shuffled: moves rvalues and binds to lvalues", "[shuffled]") { + BasicIterableWithConstDeref bi{1, 2}; + shuffled(bi); + REQUIRE_FALSE(bi.was_moved_from()); + + shuffled(std::move(bi)); + REQUIRE_FALSE(bi.was_moved_from()); // shuffled not store container inside +} + +TEST_CASE("shuffled: doesn't move or copy elements of iterable", "[shuffled]") { + using itertest::SolidInt; + constexpr SolidInt arr[] = {{6}, {7}, {8}}; + for (auto &&i : shuffled(arr)) + (void)i; +} + +template +using ImpT = decltype(shuffled(std::declval())); +TEST_CASE("shuffled: has correct ctor and assign ops", "[shuffled]") { + REQUIRE(itertest::IsMoveConstructibleOnly>::value); + REQUIRE(itertest::IsMoveConstructibleOnly>::value); +} + From 4ffacb19ba6b216b9bb519b3a4b8d48b1c7928a7 Mon Sep 17 00:00:00 2001 From: hoxnox Date: Mon, 16 May 2016 08:51:10 +0300 Subject: [PATCH 02/16] FIX: typo in README.md#shuffled --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 474b2e99..a5b405a6 100644 --- a/README.md +++ b/README.md @@ -547,7 +547,7 @@ The below outputs `0 2 4 3 1`. ```c++ unordered_set nums{4, 0, 2, 1, 3}; -for (auto&& i : sorted(nums)) { +for (auto&& i : shuffled(nums)) { cout << i << '\n'; } ``` From 2dffbd96b06eddc5b8567c9ecc7f588844796ade Mon Sep 17 00:00:00 2001 From: hoxnox Date: Wed, 18 May 2016 08:56:40 +0300 Subject: [PATCH 03/16] FIX: shuffled: mask initialization error --- shuffled.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shuffled.hpp b/shuffled.hpp index 4f7073e2..0d42316d 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -81,8 +81,8 @@ class iter::impl::ShuffledView { in_begin(std::begin(container)), seed(seed) { if (size > 0) { - uint64_t mask = 0; - std::uninitialized_fill((char*)&mask, (char*)&mask + size_approx, 0xFF); + uint64_t mask = 0xFFFFFFFFFFFFFFFFULL; + mask >> (64-size_approx); this->seed = seed & mask; this->seed = lfsr::shift(this->seed, size_approx); while(this->seed >= size) From 56e30453e5635b4591fe95d56805c4d29b708822 Mon Sep 17 00:00:00 2001 From: hoxnox Date: Wed, 18 May 2016 10:42:32 +0300 Subject: [PATCH 04/16] FIX: shuffled: efficiency improvements 1. There was a bug. When we approximate, for example 10 with power of two - we got 16 (2^4). So register size must be 4. But instead register if size 5 was used. It's not efficient. 2. Even we have efficient std::distance and operator+(int n) functions dumb_advanced and dumb_size doesn't uses them, so they was replaced with std::advance and std::distance accordingly --- shuffled.hpp | 14 ++++++++------ test/test_shuffled.cpp | 26 +++++++++++++------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/shuffled.hpp b/shuffled.hpp index 0d42316d..8a0c4cf3 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -10,7 +10,8 @@ * implemented through advance of the first (begin) iterator of the * container, so random access iterator is desirable. In the constructor * we have to calculate the total size of the container, so - * dumb_distance will be used.*/ + * std::distance will be used. User can define more effective + * std::distance for the container.*/ namespace iter { namespace impl { @@ -33,7 +34,7 @@ namespace iter { impl::ShuffledView shuffled(Container&&, int seed = 1); } -// power of 2 approximation +// power of 2 approximation (val < pow(2, get_approx(val)+1)) uint16_t iter::impl::lfsr::get_approx(uint64_t val) { if (val == 0) return 0; @@ -49,7 +50,7 @@ uint16_t iter::impl::lfsr::get_approx(uint64_t val) { break; } } - return pow2_approx + 1; + return pow2_approx; } uint64_t iter::impl::lfsr::shift(uint64_t reg, uint8_t reg_size) { @@ -77,12 +78,13 @@ class iter::impl::ShuffledView { using IterDeref = typename std::remove_reference>; ShuffledView(ShuffledView&&) : size(0) {}; ShuffledView(Container&& container, int seed) - : size(dumb_size(container)), size_approx(lfsr::get_approx(size)), + : size(std::distance(std::begin(container), std::end(container))), + size_approx(lfsr::get_approx(size)), in_begin(std::begin(container)), seed(seed) { if (size > 0) { uint64_t mask = 0xFFFFFFFFFFFFFFFFULL; - mask >> (64-size_approx); + mask = (mask >> (64-size_approx)); this->seed = seed & mask; this->seed = lfsr::shift(this->seed, size_approx); while(this->seed >= size) @@ -133,7 +135,7 @@ class iter::impl::ShuffledView { auto operator*() -> decltype(*copy) { copy = owner->in_begin; - dumb_advance(copy, static_cast(state-1)); + std::advance(copy, static_cast(state-1)); return *copy; } diff --git a/test/test_shuffled.cpp b/test/test_shuffled.cpp index e958289a..f70eacbd 100644 --- a/test/test_shuffled.cpp +++ b/test/test_shuffled.cpp @@ -19,7 +19,7 @@ TEST_CASE("shuffled: iterates through a vector in shuffled order", "[shuffled]") Vec ns = {4, 0, 5, 1, 6, 7, 9, 3, 2, 8}; auto s = shuffled(ns); Vec v(std::begin(s), std::end(s)); - Vec vc = {2, 7, 5, 9, 8, 6, 3, 1, 0, 4}; + Vec vc = {2, 9, 8, 6, 7, 5, 3, 1, 0, 4}; REQUIRE(v == vc); } @@ -41,27 +41,27 @@ TEST_CASE("shuffled: restore iteration through a vector in shuffled order " // overflow - same as default Vec v10(s.restore(10), std::end(s)); - Vec vc10 = {2, 7, 5, 9, 8, 6, 3, 1, 0, 4}; + Vec vc10 = {2, 9, 8, 6, 7, 5, 3, 1, 0, 4}; REQUIRE(v10 == vc10); - Vec v1(s.restore(5), std::end(s)); - Vec vc1 = {7, 5, 9, 8, 6, 3, 1, 0, 4}; + Vec v1(s.restore(6), std::end(s)); + Vec vc1 = {9, 8, 6, 7, 5, 3, 1, 0, 4}; REQUIRE(v1 == vc1); - Vec v2(s.restore(2), std::end(s)); - Vec vc2 = {5, 9, 8, 6, 3, 1, 0, 4}; + Vec v2(s.restore(9), std::end(s)); + Vec vc2 = {8, 6, 7, 5, 3, 1, 0, 4}; REQUIRE(v2 == vc2); - Vec v3(s.restore(6), std::end(s)); - Vec vc3 = {9, 8, 6, 3, 1, 0, 4}; + Vec v3(s.restore(4), std::end(s)); + Vec vc3 = {6, 7, 5, 3, 1, 0, 4}; REQUIRE(v3 == vc3); - Vec v4(s.restore(9), std::end(s)); - Vec vc4 = {8, 6, 3, 1, 0, 4}; + Vec v4(s.restore(5), std::end(s)); + Vec vc4 = {7, 5, 3, 1, 0, 4}; REQUIRE(v4 == vc4); - Vec v5(s.restore(4), std::end(s)); - Vec vc5 = {6, 3, 1, 0, 4}; + Vec v5(s.restore(2), std::end(s)); + Vec vc5 = {5, 3, 1, 0, 4}; REQUIRE(v5 == vc5); Vec v6(s.restore(7), std::end(s)); @@ -96,7 +96,7 @@ TEST_CASE("shuffled: iterates through a vector in shuffled order with non" Vec ns = {4, 0, 5, 1, 6, 7, 9, 3, 2, 8}; auto s = shuffled(ns, 1234); Vec v(std::begin(s), std::end(s)); - Vec vc = {9, 8, 6, 3, 1, 0, 4, 2, 7, 5}; + Vec vc = {4, 2, 9, 8, 6, 7, 5, 3, 1, 0}; REQUIRE(v == vc); } From 4b11d7cadc1c423dfd649c1f4d3e910cb008d70b Mon Sep 17 00:00:00 2001 From: hoxnox Date: Wed, 18 May 2016 15:09:39 +0300 Subject: [PATCH 05/16] FIX: shuffled: multiple lfsr funcs definitions --- shuffled.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/shuffled.hpp b/shuffled.hpp index 8a0c4cf3..fd528fae 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -35,7 +35,8 @@ namespace iter { } // power of 2 approximation (val < pow(2, get_approx(val)+1)) -uint16_t iter::impl::lfsr::get_approx(uint64_t val) { +inline uint16_t +iter::impl::lfsr::get_approx(uint64_t val) { if (val == 0) return 0; uint16_t pow2_approx = 0; @@ -53,7 +54,8 @@ uint16_t iter::impl::lfsr::get_approx(uint64_t val) { return pow2_approx; } -uint64_t iter::impl::lfsr::shift(uint64_t reg, uint8_t reg_size) { +inline uint64_t +iter::impl::lfsr::shift(uint64_t reg, uint8_t reg_size) { if (reg & 1) reg = ((reg ^ PRIME_POLY[reg_size]) >> 1) | (1 << reg_size); else From 6f5c84ae1449c82ea7abc3606f6127759c0417e7 Mon Sep 17 00:00:00 2001 From: hoxnox Date: Mon, 23 May 2016 07:54:36 +0300 Subject: [PATCH 06/16] ENH: shuffled improvements 1. Store the shuffled container internally 2. Removed unnecessary checking in operator++ 3. Distance is always uint64_t --- shuffled.hpp | 33 +++++++++++++++++---------------- test/test_shuffled.cpp | 2 +- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/shuffled.hpp b/shuffled.hpp index fd528fae..def30322 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -15,7 +15,7 @@ namespace iter { namespace impl { - template + template class ShuffledView; // linear feedback shift register namespace lfsr { @@ -30,8 +30,8 @@ namespace iter { } } - template - impl::ShuffledView shuffled(Container&&, int seed = 1); + template + impl::ShuffledView shuffled(Container&&, int seed = 1); } // power of 2 approximation (val < pow(2, get_approx(val)+1)) @@ -63,26 +63,29 @@ iter::impl::lfsr::shift(uint64_t reg, uint8_t reg_size) { return reg; } -template +template class iter::impl::ShuffledView { private: - Distance size; + uint64_t size; uint8_t size_approx; iterator_type in_begin; uint64_t seed; + Container container; - template - friend ShuffledView iter::shuffled(C&&, int); + template + friend ShuffledView iter::shuffled(C&&, int); public: using IterDeref = typename std::remove_reference>; - ShuffledView(ShuffledView&&) : size(0) {}; + ShuffledView(ShuffledView&& copy) + : size(0), container(std::forward(copy.container)) {}; ShuffledView(Container&& container, int seed) : size(std::distance(std::begin(container), std::end(container))), size_approx(lfsr::get_approx(size)), - in_begin(std::begin(container)), seed(seed) { + in_begin(std::begin(container)), seed(seed), + container(std::forward(container)) { if (size > 0) { uint64_t mask = 0xFFFFFFFFFFFFFFFFULL; @@ -97,8 +100,8 @@ class iter::impl::ShuffledView { class Iterator : public std::iterator { private: - friend class ShuffledView; - ShuffledView* owner; + friend class ShuffledView; + ShuffledView* owner; uint64_t state; iterator_type copy; // referenced by operator* value @@ -111,8 +114,6 @@ class iter::impl::ShuffledView { }; Iterator& operator++() { - if (operator==(owner->end())) - return *this; state = lfsr::shift(state, owner->size_approx); while(state > owner->size) state = lfsr::shift(state, owner->size_approx); @@ -137,7 +138,7 @@ class iter::impl::ShuffledView { auto operator*() -> decltype(*copy) { copy = owner->in_begin; - std::advance(copy, static_cast(state-1)); + std::advance(copy, static_cast(state-1)); return *copy; } @@ -179,8 +180,8 @@ class iter::impl::ShuffledView { } }; -template -iter::impl::ShuffledView iter::shuffled( +template +iter::impl::ShuffledView iter::shuffled( Container&& container, int seed = 1) { return {std::forward(container), seed}; } diff --git a/test/test_shuffled.cpp b/test/test_shuffled.cpp index f70eacbd..e07c6a46 100644 --- a/test/test_shuffled.cpp +++ b/test/test_shuffled.cpp @@ -205,7 +205,7 @@ TEST_CASE("shuffled: moves rvalues and binds to lvalues", "[shuffled]") { REQUIRE_FALSE(bi.was_moved_from()); shuffled(std::move(bi)); - REQUIRE_FALSE(bi.was_moved_from()); // shuffled not store container inside + REQUIRE(bi.was_moved_from()); } TEST_CASE("shuffled: doesn't move or copy elements of iterable", "[shuffled]") { From 5a8e90395281c60796ecb6d506268fa779cfdbb0 Mon Sep 17 00:00:00 2001 From: hoxnox Date: Mon, 23 May 2016 13:36:33 +0300 Subject: [PATCH 07/16] Bigfix: shuffled: endless cycle when size=1 --- shuffled.hpp | 6 ++++-- test/test_shuffled.cpp | 8 ++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/shuffled.hpp b/shuffled.hpp index def30322..eeec28c1 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -86,8 +86,10 @@ class iter::impl::ShuffledView { size_approx(lfsr::get_approx(size)), in_begin(std::begin(container)), seed(seed), container(std::forward(container)) { - if (size > 0) - { + if (size == 1) { + this->seed = 1; + } + else if (size > 1) { uint64_t mask = 0xFFFFFFFFFFFFFFFFULL; mask = (mask >> (64-size_approx)); this->seed = seed & mask; diff --git a/test/test_shuffled.cpp b/test/test_shuffled.cpp index e07c6a46..61ae582a 100644 --- a/test/test_shuffled.cpp +++ b/test/test_shuffled.cpp @@ -222,3 +222,11 @@ TEST_CASE("shuffled: has correct ctor and assign ops", "[shuffled]") { REQUIRE(itertest::IsMoveConstructibleOnly>::value); } +TEST_CASE("shuffled: correct work with 1 and 0 sized container.", "[shuffled]") { + Vec onesz = {7}; + for (auto &&i : shuffled(onesz)) + REQUIRE(i == 7); + Vec zerosz; + for (auto &&i : shuffled(zerosz)) + REQUIRE(false); +} From aa0594e93869c6a4bb797019985bf15189fdf5e3 Mon Sep 17 00:00:00 2001 From: hoxnox Date: Wed, 25 May 2016 11:52:47 +0300 Subject: [PATCH 08/16] Bugfix: overflow on size >= 2^32 --- shuffled.hpp | 15 +++++++++------ test/test_shuffled.cpp | 18 +++++++++++++++++- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/shuffled.hpp b/shuffled.hpp index eeec28c1..b80c26ca 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -38,7 +38,7 @@ namespace iter { inline uint16_t iter::impl::lfsr::get_approx(uint64_t val) { if (val == 0) - return 0; + return 0; uint16_t pow2_approx = 0; if (val > 9223372036854775808UL) { pow2_approx = 63; @@ -56,10 +56,13 @@ iter::impl::lfsr::get_approx(uint64_t val) { inline uint64_t iter::impl::lfsr::shift(uint64_t reg, uint8_t reg_size) { - if (reg & 1) - reg = ((reg ^ PRIME_POLY[reg_size]) >> 1) | (1 << reg_size); - else + if (reg & 1) { + reg = ((reg ^ PRIME_POLY[reg_size]) >> 1) | + (static_cast(1) << reg_size); + } + else { reg >>= 1; + } return reg; } @@ -91,10 +94,10 @@ class iter::impl::ShuffledView { } else if (size > 1) { uint64_t mask = 0xFFFFFFFFFFFFFFFFULL; - mask = (mask >> (64-size_approx)); + mask = (mask >> (64-(size_approx+1))); this->seed = seed & mask; this->seed = lfsr::shift(this->seed, size_approx); - while(this->seed >= size) + while(this->seed > size) this->seed = lfsr::shift(this->seed, size_approx); } } diff --git a/test/test_shuffled.cpp b/test/test_shuffled.cpp index 61ae582a..7a671b98 100644 --- a/test/test_shuffled.cpp +++ b/test/test_shuffled.cpp @@ -23,6 +23,22 @@ TEST_CASE("shuffled: iterates through a vector in shuffled order", "[shuffled]") REQUIRE(v == vc); } +TEST_CASE("shuffled: shuffling one-element container", "[shuffled]") { + Vec ns = {4}; + auto s = shuffled(ns); + Vec v(std::begin(s), std::end(s)); + Vec vc = {4}; + REQUIRE(v == vc); +} + +TEST_CASE("shuffled: shuffling two-element container", "[shuffled]") { + Vec ns = {4, 3}; + auto s = shuffled(ns); + Vec v(std::begin(s), std::end(s)); + Vec vc = {3, 4}; + REQUIRE(v == vc); +} + TEST_CASE("shuffled: iterates through a list of strings in shuffled order", "[shuffled]") { std::list ns = {"apple", "banana", "orange", "peach"}; @@ -114,7 +130,7 @@ TEST_CASE("shuffled: can iterate over unordered container", "[shuffled]") { auto s = shuffled(ns); Vec v(std::begin(s), std::end(s)); - Vec vc = {3, 2, 1, 4, 0}; + Vec vc = {0, 3, 2, 1, 4}; REQUIRE(v == vc); } From e0dae96a3a32b8dee2b01483270ef730a7f6583a Mon Sep 17 00:00:00 2001 From: hoxnox Date: Wed, 25 May 2016 15:03:10 +0300 Subject: [PATCH 09/16] Bigfix: constant truncation on 32-bit system --- shuffled.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shuffled.hpp b/shuffled.hpp index b80c26ca..c93d077d 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -40,12 +40,12 @@ iter::impl::lfsr::get_approx(uint64_t val) { if (val == 0) return 0; uint16_t pow2_approx = 0; - if (val > 9223372036854775808UL) { + if (val > 9223372036854775808ULL) { pow2_approx = 63; } else { while (pow2_approx < 64) { - if (val >= (1UL << (pow2_approx + 1))) + if (val >= (static_cast(1) << (pow2_approx + 1))) ++pow2_approx; else break; From a7399f6b78925aa2f176eed2900e4ff893994b3e Mon Sep 17 00:00:00 2001 From: hoxnox Date: Thu, 24 Oct 2019 14:00:39 +0300 Subject: [PATCH 10/16] TEST: actualized, warn fixed --- test/test_shuffled.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/test_shuffled.cpp b/test/test_shuffled.cpp index 7a671b98..c865d559 100644 --- a/test/test_shuffled.cpp +++ b/test/test_shuffled.cpp @@ -130,7 +130,7 @@ TEST_CASE("shuffled: can iterate over unordered container", "[shuffled]") { auto s = shuffled(ns); Vec v(std::begin(s), std::end(s)); - Vec vc = {0, 3, 2, 1, 4}; + Vec vc = {1, 2, 3, 0, 4}; REQUIRE(v == vc); } @@ -244,5 +244,8 @@ TEST_CASE("shuffled: correct work with 1 and 0 sized container.", "[shuffled]") REQUIRE(i == 7); Vec zerosz; for (auto &&i : shuffled(zerosz)) + { + (void)i; // prevent warning REQUIRE(false); + } } From 173db41075694db9319c90840753ef4e3051110f Mon Sep 17 00:00:00 2001 From: hoxnox Date: Thu, 24 Oct 2019 14:23:06 +0300 Subject: [PATCH 11/16] FIX: deprecated implicit constructor --- shuffled.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/shuffled.hpp b/shuffled.hpp index c93d077d..7276851d 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -112,6 +112,7 @@ class iter::impl::ShuffledView { public: Iterator() : owner(nullptr), state(0) {} + Iterator(const Iterator& other) { operator=(other); } Iterator& operator=(const Iterator& other) { owner = other.owner; state = other.state; From 6946a0bf6690451f8bc4b1ddf0edf0970464e6e6 Mon Sep 17 00:00:00 2001 From: hoxnox Date: Thu, 24 Oct 2019 14:50:43 +0300 Subject: [PATCH 12/16] FIX: advance replaced with next to conform ForwardIterator requirements --- shuffled.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shuffled.hpp b/shuffled.hpp index 7276851d..5258c325 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -144,7 +144,7 @@ class iter::impl::ShuffledView { auto operator*() -> decltype(*copy) { copy = owner->in_begin; - std::advance(copy, static_cast(state-1)); + copy = std::next(copy, static_cast(state-1)); return *copy; } From acf3edcf4d3693149a5b8bf7b13aee199ba2c1a8 Mon Sep 17 00:00:00 2001 From: hoxnox Date: Thu, 13 Oct 2016 15:34:22 +0300 Subject: [PATCH 13/16] FIX: default value in realisation --- shuffled.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shuffled.hpp b/shuffled.hpp index 5258c325..4fa28987 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -188,7 +188,7 @@ class iter::impl::ShuffledView { template iter::impl::ShuffledView iter::shuffled( - Container&& container, int seed = 1) { + Container&& container, int seed /*= 1*/) { return {std::forward(container), seed}; } From aab789a508c738915127c6e379a57958ec8937f9 Mon Sep 17 00:00:00 2001 From: hoxnox Date: Fri, 25 Oct 2019 15:29:46 +0300 Subject: [PATCH 14/16] FIX: not obvious Iterator type, clang is not happy --- shuffled.hpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/shuffled.hpp b/shuffled.hpp index 4fa28987..460aebbe 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -102,8 +102,7 @@ class iter::impl::ShuffledView { } } - class Iterator - : public std::iterator { + class Iterator { private: friend class ShuffledView; ShuffledView* owner; @@ -111,6 +110,12 @@ class iter::impl::ShuffledView { iterator_type copy; // referenced by operator* value public: + using iterator_category = std::input_iterator_tag; + using value_type = iterator_traits_deref; + using difference_type = std::ptrdiff_t; + using pointer = value_type*; + using reference = value_type&; + Iterator() : owner(nullptr), state(0) {} Iterator(const Iterator& other) { operator=(other); } Iterator& operator=(const Iterator& other) { @@ -142,13 +147,13 @@ class iter::impl::ShuffledView { return !operator==(other); } - auto operator*() -> decltype(*copy) { + auto& operator*() { copy = owner->in_begin; copy = std::next(copy, static_cast(state-1)); return *copy; } - ArrowProxy operator->() { + auto operator->() -> ArrowProxy{ return {**this}; } }; From 53a02508b350759a986c10c07e109d4dce2b589b Mon Sep 17 00:00:00 2001 From: hoxnox Date: Fri, 25 Oct 2019 15:56:47 +0300 Subject: [PATCH 15/16] FIX: redeclaration brakes default seed, VS2017 is not happy --- shuffled.hpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/shuffled.hpp b/shuffled.hpp index 460aebbe..de86f5e3 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -31,7 +31,9 @@ namespace iter { } template - impl::ShuffledView shuffled(Container&&, int seed = 1); + impl::ShuffledView shuffled(Container&& container, int seed = 1) { + return {std::forward(container), seed}; + } } // power of 2 approximation (val < pow(2, get_approx(val)+1)) @@ -191,11 +193,5 @@ class iter::impl::ShuffledView { } }; -template -iter::impl::ShuffledView iter::shuffled( - Container&& container, int seed /*= 1*/) { - return {std::forward(container), seed}; -} - #endif From f872b6542c9aeec74ee978c275c41091892e16cf Mon Sep 17 00:00:00 2001 From: hoxnox Date: Fri, 25 Oct 2019 16:26:23 +0300 Subject: [PATCH 16/16] FIX: gcc-9.* wants bidir iterator even for std::next --- shuffled.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shuffled.hpp b/shuffled.hpp index de86f5e3..73919fc9 100644 --- a/shuffled.hpp +++ b/shuffled.hpp @@ -149,9 +149,9 @@ class iter::impl::ShuffledView { return !operator==(other); } - auto& operator*() { + auto operator*() -> decltype(*copy) { copy = owner->in_begin; - copy = std::next(copy, static_cast(state-1)); + dumb_advance(copy, std::end(owner->container), static_cast(state-1)); return *copy; }