Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] seqan3::views::single_pass_input::iterator::operator++(int) must return void (real cxx20 std::input_iterator) #2775

Merged
merged 7 commits into from
Aug 24, 2021
19 changes: 14 additions & 5 deletions include/seqan3/io/views/detail/take_exactly_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,21 @@ class view_take_exactly<urng_t, or_throw>::basic_iterator :
}

//!\brief Returns an iterator incremented by one.
constexpr basic_iterator operator++(int) noexcept(noexcept(++std::declval<basic_iterator &>()) &&
std::is_nothrow_copy_constructible_v<basic_iterator>)
constexpr decltype(auto) operator++(int) noexcept(noexcept(++std::declval<basic_iterator &>()) &&
(std::same_as<decltype(std::declval<base_base_t &>()++), void> ||
std::is_nothrow_copy_constructible_v<basic_iterator>))
{
basic_iterator cpy{*this};
++(*this);
return cpy;
// if underlying iterator is a C++20 input iterator (i.e. returns void), return void too.
if constexpr (std::same_as<decltype(std::declval<base_base_t &>()++), void>)
{
++(*this);
}
else
{
basic_iterator cpy{*this};
++(*this);
return cpy;
}
}

//!\brief Decrements the iterator by one.
Expand Down
19 changes: 14 additions & 5 deletions include/seqan3/io/views/detail/take_until_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,13 +284,22 @@ class view_take_until<urng_t, fun_t, or_throw, and_consume>::basic_consume_itera
}

//!\brief Post-increment implemented via pre-increment.
basic_consume_iterator operator++(int)
decltype(auto) operator++(int)
noexcept(noexcept(++std::declval<basic_consume_iterator &>()) &&
std::is_nothrow_copy_constructible_v<basic_consume_iterator>)
(std::same_as<decltype(std::declval<underlying_iterator_t &>()++), void> ||
std::is_nothrow_copy_constructible_v<basic_consume_iterator>))
{
basic_consume_iterator cpy{*this};
++(*this);
return cpy;
// if underlying iterator is a C++20 input iterator (i.e. returns void), return void too.
if constexpr (std::same_as<decltype(std::declval<underlying_iterator_t &>()++), void>)
{
++(*this);
}
else
{
basic_consume_iterator cpy{*this};
++(*this);
return cpy;
}
}
//!\}
/*!\name Comparison operators
Expand Down
20 changes: 7 additions & 13 deletions include/seqan3/utility/views/single_pass_input.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,19 +246,13 @@ class basic_iterator<single_pass_input_view<view_type>>
}

//!\brief Post-increment.
auto operator++(int) noexcept
void operator++(int) noexcept
{
if constexpr (std::output_iterator<base_iterator_type, reference> &&
std::copy_constructible<base_iterator_type>)
{
basic_iterator tmp{*this};
++(*this);
return tmp;
}
else
{
++(*this);
}
// this post-increment can't be an std::output_iterator, because it would require that `*it++ = value` must have
// the same semantic as `*i = value; ++i`, but it actually has the following `++i; *i = value` semantic, due to
// the centralised storage of the underlying_iterator in the view where each copy of a basic_iterator points to
// the same centralised state.
++(*this);
}
//!\}

Expand Down Expand Up @@ -342,7 +336,7 @@ namespace seqan3::views
* | std::ranges::view | | *guaranteed* |
* | std::ranges::sized_range | | *lost* |
* | std::ranges::common_range | | *lost* |
* | std::ranges::output_range | | *preserved* |
* | std::ranges::output_range | | *lost* |
* | seqan3::const_iterable_range | | *lost* |
* | | | |
* | std::ranges::range_reference_t | | std::ranges::range_reference_t<urng_t> |
Expand Down
2 changes: 1 addition & 1 deletion test/unit/io/views/detail/take_exactly_view_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ void do_concepts(adaptor_t && adaptor, bool const exactly)
EXPECT_EQ(std::ranges::sized_range<decltype(v2)>, exactly);
EXPECT_FALSE(std::ranges::common_range<decltype(v2)>);
EXPECT_FALSE(seqan3::const_iterable_range<decltype(v2)>);
EXPECT_TRUE((std::ranges::output_range<decltype(v2), int>));
EXPECT_FALSE((std::ranges::output_range<decltype(v2), int>));

// explicit test for non const-iterable views
// https://github.com/seqan/seqan3/pull/1734#discussion_r408829267
Expand Down
2 changes: 1 addition & 1 deletion test/unit/io/views/detail/take_line_view_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void do_concepts(adaptor_t const & adaptor)
EXPECT_FALSE(std::ranges::sized_range<decltype(v2)>);
EXPECT_FALSE(std::ranges::common_range<decltype(v2)>);
EXPECT_FALSE(seqan3::const_iterable_range<decltype(v2)>);
EXPECT_TRUE((std::ranges::output_range<decltype(v2), char>));
EXPECT_FALSE((std::ranges::output_range<decltype(v2), char>));
}

// ============================================================================
Expand Down
2 changes: 1 addition & 1 deletion test/unit/io/views/detail/take_until_view_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void do_concepts(adaptor_t && adaptor, bool const_it)
EXPECT_FALSE(std::ranges::sized_range<decltype(v2)>);
EXPECT_FALSE(std::ranges::common_range<decltype(v2)>);
EXPECT_FALSE(seqan3::const_iterable_range<decltype(v2)>);
EXPECT_TRUE((std::ranges::output_range<decltype(v2), char>));
EXPECT_FALSE((std::ranges::output_range<decltype(v2), char>)); // lost by single_pass_input

// explicit test for non const-iterable views
// https://github.com/seqan/seqan3/pull/1734#discussion_r408829267
Expand Down
3 changes: 1 addition & 2 deletions test/unit/utility/views/single_pass_input_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ TYPED_TEST(single_pass_input, view_concept)
EXPECT_TRUE(std::ranges::range<view_t>);
EXPECT_TRUE(std::ranges::view<view_t>);
EXPECT_TRUE(std::ranges::input_range<view_t>);
EXPECT_EQ((std::ranges::output_range<view_t, std::ranges::range_reference_t<view_t>>),
(std::ranges::output_range<rng_t, std::ranges::range_reference_t<rng_t>>));
EXPECT_FALSE((std::ranges::output_range<view_t, std::ranges::range_reference_t<view_t>>));
EXPECT_FALSE(std::ranges::common_range<view_t>);
EXPECT_FALSE(std::ranges::forward_range<view_t>);
EXPECT_FALSE(std::ranges::bidirectional_range<view_t>);
Expand Down
2 changes: 1 addition & 1 deletion test/unit/utility/views/slice_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ TEST(view_slice, concepts)
EXPECT_EQ(std::ranges::sized_range<decltype(v2)>, false);
EXPECT_FALSE(std::ranges::common_range<decltype(v2)>);
EXPECT_FALSE(seqan3::const_iterable_range<decltype(v2)>);
EXPECT_TRUE((std::ranges::output_range<decltype(v2), int>));
EXPECT_FALSE((std::ranges::output_range<decltype(v2), int>)); // single_pass_input loses it
}

TEST(view_slice, underlying_is_shorter)
Expand Down