From bfbe6d21e224cc0661b3dafdac339133c294aeb3 Mon Sep 17 00:00:00 2001 From: Tristan Brindle Date: Sun, 29 Dec 2024 16:52:54 +0000 Subject: [PATCH 1/2] Fix issue #228 Manually implement `set_symmetric_difference_adaptor::cursor_type`'s `operator==` so that it doesn't compare the `state_` field Fixes #228 --- include/flux/adaptor/set_adaptors.hpp | 37 +++++++++++++---------- test/test_set_adaptors.cpp | 43 ++++++++++++++++++++++----- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/include/flux/adaptor/set_adaptors.hpp b/include/flux/adaptor/set_adaptors.hpp index 5bb1a731..80ceb366 100644 --- a/include/flux/adaptor/set_adaptors.hpp +++ b/include/flux/adaptor/set_adaptors.hpp @@ -74,7 +74,7 @@ struct set_union_adaptor public: using value_type = std::common_type_t, value_t>; - inline static constexpr bool is_infinite = flux::infinite_sequence || + inline static constexpr bool is_infinite = flux::infinite_sequence || flux::infinite_sequence; template @@ -91,7 +91,7 @@ struct set_union_adaptor requires maybe_const_iterable static constexpr auto is_last(Self& self, cursor_type const& cur) -> bool { - return flux::is_last(self.base1_, cur.base1_cursor) && + return flux::is_last(self.base1_, cur.base1_cursor) && flux::is_last(self.base2_, cur.base2_cursor); } @@ -111,7 +111,7 @@ struct set_union_adaptor template requires maybe_const_iterable static constexpr auto read_at(Self& self, cursor_type const& cur) - -> std::common_reference_t std::common_reference_t { if (cur.active_ == cursor_type::first) { @@ -122,7 +122,7 @@ struct set_union_adaptor } template - requires maybe_const_iterable && + requires maybe_const_iterable && bounded_sequence && bounded_sequence static constexpr auto last(Self& self) -> cursor_type { @@ -132,7 +132,7 @@ struct set_union_adaptor template requires maybe_const_iterable static constexpr auto move_at(Self& self, cursor_type const& cur) - -> std::common_reference_t std::common_reference_t { if (cur.active_ == cursor_type::first) { @@ -141,7 +141,7 @@ struct set_union_adaptor return flux::move_at(self.base2_, cur.base2_cursor); } } - + }; }; @@ -244,7 +244,7 @@ struct set_difference_adaptor { return flux::move_at(self.base1_, cur.base1_cursor); } - + }; }; @@ -272,10 +272,13 @@ struct set_symmetric_difference_adaptor cursor_t base2_cursor; enum : char {first, second, first_done, second_done} state_ = first; - friend auto operator==(cursor_type const&, cursor_type const&) -> bool + friend constexpr auto operator==(cursor_type const& lhs, cursor_type const& rhs) -> bool requires std::equality_comparable> && std::equality_comparable> - = default; + { + return lhs.base1_cursor == rhs.base1_cursor && + lhs.base2_cursor == rhs.base2_cursor; + } }; template @@ -311,7 +314,7 @@ struct set_symmetric_difference_adaptor public: using value_type = std::common_type_t, value_t>; - inline static constexpr bool is_infinite = flux::infinite_sequence || + inline static constexpr bool is_infinite = flux::infinite_sequence || flux::infinite_sequence; template @@ -328,7 +331,7 @@ struct set_symmetric_difference_adaptor requires maybe_const_iterable static constexpr auto is_last(Self& self, cursor_type const& cur) -> bool { - return flux::is_last(self.base1_, cur.base1_cursor) && + return flux::is_last(self.base1_, cur.base1_cursor) && flux::is_last(self.base2_, cur.base2_cursor); } @@ -370,7 +373,9 @@ struct set_symmetric_difference_adaptor } template - requires maybe_const_iterable && bounded_sequence + requires maybe_const_iterable && + bounded_sequence && + bounded_sequence static constexpr auto last(Self& self) -> cursor_type { return cursor_type{flux::last(self.base1_), flux::last(self.base2_)}; @@ -428,7 +433,7 @@ struct set_intersection_adaptor template static constexpr void update(Self& self, cursor_type& cur) { - while(not flux::is_last(self.base1_, cur.base1_cursor) && + while(not flux::is_last(self.base1_, cur.base1_cursor) && not flux::is_last(self.base2_, cur.base2_cursor)) { auto r = std::invoke(self.cmp_, flux::read_at(self.base1_, cur.base1_cursor), @@ -463,7 +468,7 @@ struct set_intersection_adaptor requires maybe_const_iterable static constexpr auto is_last(Self& self, cursor_type const& cur) -> bool { - return flux::is_last(self.base1_, cur.base1_cursor) || + return flux::is_last(self.base1_, cur.base1_cursor) || flux::is_last(self.base2_, cur.base2_cursor); } @@ -491,7 +496,7 @@ struct set_intersection_adaptor { return flux::move_at(self.base1_, cur.base1_cursor); } - + }; }; @@ -503,7 +508,7 @@ concept set_op_compatible = struct set_union_fn { template - requires set_op_compatible && + requires set_op_compatible && weak_ordering_for && weak_ordering_for [[nodiscard]] diff --git a/test/test_set_adaptors.cpp b/test/test_set_adaptors.cpp index b52ad197..5394b77b 100644 --- a/test/test_set_adaptors.cpp +++ b/test/test_set_adaptors.cpp @@ -110,8 +110,9 @@ constexpr bool test_set_union() std::array, 3> arr1{{{0, 'a'}, {2, 'b'}, {4, 'c'}}}; std::array, 3> arr2{{{1, 'x'}, {3, 'y'}, {5, 'z'}}}; - auto union_seq = flux::set_union(flux::ref(arr1), flux::ref(arr2), - flux::proj(flux::cmp::compare, [] (auto v) { return v.first; })); + auto union_seq + = flux::set_union(flux::ref(arr1), flux::ref(arr2), + flux::proj(flux::cmp::compare, [](auto v) { return v.first; })); using T = decltype(union_seq); static_assert(flux::sequence); @@ -267,8 +268,9 @@ constexpr bool test_set_difference() std::array, 4> arr1{{{0, 'a'}, {1, 'b'}, {2, 'c'}, {3, 'd'}}}; std::array, 3> arr2{{{1, 'x'}, {2, 'y'}, {5, 'z'}}}; - auto diff_seq = flux::set_difference(flux::ref(arr1), flux::ref(arr2), - flux::proj(flux::cmp::compare, [] (auto v) { return v.first; })); + auto diff_seq + = flux::set_difference(flux::ref(arr1), flux::ref(arr2), + flux::proj(flux::cmp::compare, [](auto v) { return v.first; })); using T = decltype(diff_seq); static_assert(flux::sequence); @@ -412,8 +414,9 @@ constexpr bool test_set_symmetric_difference() std::array, 4> arr1{{{0, 'a'}, {1, 'b'}, {2, 'c'}, {3, 'd'}}}; std::array, 3> arr2{{{1, 'x'}, {2, 'y'}, {5, 'z'}}}; - auto diff_seq = flux::set_symmetric_difference(flux::ref(arr1), flux::ref(arr2), - flux::proj(flux::cmp::compare, [] (auto v) { return v.first; })); + auto diff_seq = flux::set_symmetric_difference( + flux::ref(arr1), flux::ref(arr2), + flux::proj(flux::cmp::compare, [](auto v) { return v.first; })); using T = decltype(diff_seq); static_assert(flux::sequence); @@ -558,8 +561,9 @@ constexpr bool test_set_intersection() std::array, 4> arr1{{{0, 'a'}, {1, 'b'}, {2, 'c'}, {3, 'd'}}}; std::array, 3> arr2{{{1, 'x'}, {2, 'y'}, {5, 'z'}}}; - auto inter_seq = flux::set_intersection(flux::ref(arr1), flux::ref(arr2), - flux::proj(flux::cmp::compare, [] (auto v) { return v.first; })); + auto inter_seq = flux::set_intersection( + flux::ref(arr1), flux::ref(arr2), + flux::proj(flux::cmp::compare, [](auto v) { return v.first; })); using T = decltype(inter_seq); static_assert(flux::sequence); @@ -607,6 +611,27 @@ static_assert(test_set_difference()); static_assert(test_set_symmetric_difference()); static_assert(test_set_intersection()); +// https://github.com/tcbrindle/flux/issues/228 +constexpr bool issue_228() +{ + std::array arr1 {0, 1, 2, 3, 4, 5}; + std::array arr2 {1, 3, 5, 6, 7}; + auto merged = flux::set_symmetric_difference(flux::ref(arr1), flux::ref(arr2)); + + STATIC_CHECK(flux::equal(merged, std::array {0, 2, 4, 6, 7})); + + std::vector out; + + for (auto i : merged) { + // crashes here + out.push_back(i); + } + + STATIC_CHECK(check_equal(out, merged)); + + return true; +} +static_assert(issue_228()); } TEST_CASE("set_union") @@ -625,6 +650,8 @@ TEST_CASE("set_symmetric_difference") { bool result = test_set_symmetric_difference(); REQUIRE(result); + result = issue_228(); + REQUIRE(result); } TEST_CASE("set_intersection") From a4b9bb6dfd6115e0e8a78cfd52be276d25ecd395 Mon Sep 17 00:00:00 2001 From: Tristan Brindle Date: Sun, 29 Dec 2024 17:00:19 +0000 Subject: [PATCH 2/2] Add missing #include --- test/test_set_adaptors.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_set_adaptors.cpp b/test/test_set_adaptors.cpp index 5394b77b..5e89d73a 100644 --- a/test/test_set_adaptors.cpp +++ b/test/test_set_adaptors.cpp @@ -5,6 +5,7 @@ #include #include +#include #include "test_utils.hpp"