Skip to content

Commit

Permalink
Merge pull request #2775 from marehr/clang03
Browse files Browse the repository at this point in the history
 [FIX] seqan3::views::single_pass_input::iterator::operator++(int) must return void (real cxx20 std::input_iterator)
  • Loading branch information
eseiler authored Aug 24, 2021
2 parents e6ed2e3 + d9b0405 commit 1df9bec
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 29 deletions.
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

1 comment on commit 1df9bec

@vercel
Copy link

@vercel vercel bot commented on 1df9bec Aug 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.