Fix and refactor buffers_cat:

Elements of zero size are correctly skipped during iteration.

* Tidy up tests
* Increase code coverage
* Remove move special members
* Correct behavior for default constructed iterators
This commit is contained in:
Vinnie Falco
2018-12-14 20:25:38 -08:00
parent ca728e4022
commit 719fe66b77
4 changed files with 350 additions and 380 deletions

View File

@@ -5,6 +5,7 @@ Version 200
* buffers_cat fixes and coverage
* Refactor buffers_adaptor
* Refactor buffers_range
* Fix and refactor buffers_cat
API Changes:

View File

@@ -46,15 +46,9 @@ public:
/// Copy Constructor
buffers_cat_view(buffers_cat_view const&) = default;
/// Move Constructor
buffers_cat_view(buffers_cat_view&&) = default;
/// Copy Assignment
buffers_cat_view& operator=(buffers_cat_view const&) = default;
/// Move Assignment
buffers_cat_view& operator=(buffers_cat_view&&) = default;
/** Constructor
@param buffers The list of buffer sequences to concatenate.

View File

@@ -35,6 +35,7 @@ struct buffers_cat_view_iterator_base
net::mutable_buffer
operator*() const
{
// Dereferencing a one-past-the-end iterator
BOOST_THROW_EXCEPTION(std::logic_error{
"invalid iterator"});
}
@@ -77,9 +78,7 @@ public:
std::bidirectional_iterator_tag;
const_iterator() = default;
const_iterator(const_iterator&& other) = default;
const_iterator(const_iterator const& other) = default;
const_iterator& operator=(const_iterator&& other) = default;
const_iterator& operator=(const_iterator const& other) = default;
bool
@@ -103,11 +102,9 @@ public:
const_iterator
operator++(int);
// deprecated
const_iterator&
operator--();
// deprecated
const_iterator
operator--(int);
@@ -115,53 +112,6 @@ private:
const_iterator(
detail::tuple<Bn...> const& bn, bool at_end);
template<std::size_t I>
void
next(C<I> const&)
{
if(net::buffer_size(
detail::get<I>(*bn_)) != 0)
{
it_.template emplace<I+1>(
net::buffer_sequence_begin(
detail::get<I>(*bn_)));
return;
}
next(C<I+1>{});
}
void
next(C<sizeof...(Bn)> const&)
{
// end
auto constexpr I = sizeof...(Bn);
it_.template emplace<I+1>();
}
template<std::size_t I>
void
prev(C<I> const&)
{
if(net::buffer_size(
detail::get<I>(*bn_)) != 0)
{
it_.template emplace<I+1>(
net::buffer_sequence_end(
detail::get<I>(*bn_)));
return;
}
prev(C<I-1>{});
}
void
prev(C<0> const&)
{
auto constexpr I = 0;
it_.template emplace<I+1>(
net::buffer_sequence_end(
detail::get<I>(*bn_)));
}
struct dereference
{
const_iterator const& self;
@@ -190,15 +140,107 @@ private:
void
operator()(mp11::mp_size_t<0>)
{
// Incrementing a default-constructed iterator
BOOST_THROW_EXCEPTION(std::logic_error{
"invalid iterator"});
}
template<std::size_t I>
void
operator()(mp11::mp_size_t<I>)
{
++self.it_.template get<I>();
next(mp11::mp_size_t<I>{});
}
template<std::size_t I>
void
next(mp11::mp_size_t<I>)
{
auto& it = self.it_.template get<I>();
for(;;)
{
if (it == net::buffer_sequence_end(
detail::get<I-1>(*self.bn_)))
break;
if(net::const_buffer(*it).size() > 0)
return;
++it;
}
self.it_.template emplace<I+1>(
net::buffer_sequence_begin(
detail::get<I>(*self.bn_)));
next(mp11::mp_size_t<I+1>{});
}
void
operator()(mp11::mp_size_t<sizeof...(Bn)>)
{
auto constexpr I = sizeof...(Bn);
++self.it_.template get<I>();
next(mp11::mp_size_t<I>{});
}
void
next(mp11::mp_size_t<sizeof...(Bn)>)
{
auto constexpr I = sizeof...(Bn);
auto& it = self.it_.template get<I>();
for(;;)
{
if (it == net::buffer_sequence_end(
detail::get<I-1>(*self.bn_)))
break;
if(net::const_buffer(*it).size() > 0)
return;
++it;
}
// end
self.it_.template emplace<I+1>();
}
[[noreturn]]
void
operator()(mp11::mp_size_t<sizeof...(Bn) + 1>)
operator()(mp11::mp_size_t<sizeof...(Bn)+1>)
{
(*this)(mp11::mp_size_t<0>{});
// Incrementing a one-past-the-end iterator
BOOST_THROW_EXCEPTION(std::logic_error{
"invalid iterator"});
}
};
struct decrement
{
const_iterator& self;
[[noreturn]]
void
operator()(mp11::mp_size_t<0>)
{
// Decrementing a default-constructed iterator
BOOST_THROW_EXCEPTION(std::logic_error{
"invalid iterator"});
}
void
operator()(mp11::mp_size_t<1>)
{
auto constexpr I = 1;
auto& it = self.it_.template get<I>();
for(;;)
{
if(it == net::buffer_sequence_begin(
detail::get<I-1>(*self.bn_)))
{
// Decrementing an iterator to the beginning
BOOST_THROW_EXCEPTION(std::logic_error{
"invalid iterator"});
}
--it;
if(net::const_buffer(*it).size() > 0)
return;
}
}
template<std::size_t I>
@@ -206,53 +248,31 @@ private:
operator()(mp11::mp_size_t<I>)
{
auto& it = self.it_.template get<I>();
if (++it == net::buffer_sequence_end(
detail::get<I - 1>(*self.bn_)))
self.next(C<I>());
for(;;)
{
if(it == net::buffer_sequence_begin(
detail::get<I-1>(*self.bn_)))
break;
--it;
if(net::const_buffer(*it).size() > 0)
return;
}
self.it_.template emplace<I-1>(
net::buffer_sequence_end(
detail::get<I-2>(*self.bn_)));
(*this)(mp11::mp_size_t<I-1>{});
}
void
operator()(mp11::mp_size_t<sizeof...(Bn)+1>)
{
auto constexpr I = sizeof...(Bn)+1;
self.it_.template emplace<I-1>(
net::buffer_sequence_end(
detail::get<I-2>(*self.bn_)));
(*this)(mp11::mp_size_t<I-1>{});
}
};
void
decrement(C<sizeof...(Bn)> const&)
{
auto constexpr I = sizeof...(Bn);
if(it_.index() == I+1)
prev(C<I-1>{});
decrement(C<I-1>{});
}
template<std::size_t I>
void
decrement(C<I> const&)
{
if(it_.index() == I+1)
{
if(it_.template get<I+1>() !=
net::buffer_sequence_begin(
detail::get<I>(*bn_)))
{
--it_.template get<I+1>();
return;
}
prev(C<I-1>{});
}
decrement(C<I-1>{});
}
void
decrement(C<0> const&)
{
auto constexpr I = 0;
if(it_.template get<I+1>() !=
net::buffer_sequence_begin(
detail::get<I>(*bn_)))
{
--it_.template get<I+1>();
return;
}
BOOST_THROW_EXCEPTION(std::logic_error{
"invalid iterator"});
}
};
//------------------------------------------------------------------------------
@@ -265,9 +285,17 @@ const_iterator(
: bn_(&bn)
{
if(! at_end)
next(C<0>{});
{
it_.template emplace<1>(
net::buffer_sequence_begin(
detail::get<0>(*bn_)));
increment{*this}.next(
mp11::mp_size_t<1>{});
}
else
next(C<sizeof...(Bn)>{});
{
it_.template emplace<sizeof...(Bn)+1>();
}
}
template<class... Bn>
@@ -276,20 +304,7 @@ buffers_cat_view<Bn...>::
const_iterator::
operator==(const_iterator const& other) const
{
return
(bn_ == nullptr) ?
(
other.bn_ == nullptr ||
other.it_.index() == sizeof...(Bn) + 1
):(
(other.bn_ == nullptr) ?
(
it_.index() == sizeof...(Bn) + 1
): (
bn_ == other.bn_ &&
it_ == other.it_
)
);
return bn_ == other.bn_ && it_ == other.it_;
}
template<class... Bn>
@@ -338,7 +353,10 @@ const_iterator::
operator--() ->
const_iterator&
{
decrement(C<sizeof...(Bn)>{});
mp11::mp_with_index<
sizeof...(Bn) + 2>(
it_.index(),
decrement{*this});
return *this;
}
@@ -365,7 +383,6 @@ buffers_cat_view(Bn const&... bn)
template<class... Bn>
inline
auto
buffers_cat_view<Bn...>::begin() const ->
const_iterator
@@ -374,7 +391,6 @@ buffers_cat_view<Bn...>::begin() const ->
}
template<class... Bn>
inline
auto
buffers_cat_view<Bn...>::end() const->
const_iterator

View File

@@ -10,6 +10,8 @@
// Test that header file is self-contained.
#include <boost/beast/core/buffers_cat.hpp>
#include "buffer_test.hpp"
#include <boost/beast/_experimental/unit_test/suite.hpp>
#include <boost/beast/core/buffers_prefix.hpp>
#include <boost/beast/core/buffers_suffix.hpp>
@@ -26,213 +28,17 @@ namespace beast {
class buffers_cat_test : public unit_test::suite
{
public:
template<class Iterator>
static
std::reverse_iterator<Iterator>
make_reverse_iterator(Iterator i)
{
return std::reverse_iterator<Iterator>(i);
}
template<class ConstBufferSequence>
static
std::size_t
bsize1(ConstBufferSequence const& bs)
buffers_length(
ConstBufferSequence const& buffers)
{
using net::buffer_size;
std::size_t n = 0;
for(auto it = bs.begin(); it != bs.end(); ++it)
n += buffer_size(*it);
return n;
return std::distance(
net::buffer_sequence_begin(buffers),
net::buffer_sequence_end(buffers));
}
template<class ConstBufferSequence>
static
std::size_t
bsize2(ConstBufferSequence const& bs)
{
using net::buffer_size;
std::size_t n = 0;
for(auto it = bs.begin(); it != bs.end(); it++)
n += buffer_size(*it);
return n;
}
template<class ConstBufferSequence>
static
std::size_t
bsize3(ConstBufferSequence const& bs)
{
using net::buffer_size;
std::size_t n = 0;
for(auto it = bs.end(); it != bs.begin();)
n += buffer_size(*--it);
return n;
}
template<class ConstBufferSequence>
static
std::size_t
bsize4(ConstBufferSequence const& bs)
{
using net::buffer_size;
std::size_t n = 0;
for(auto it = bs.end(); it != bs.begin();)
{
it--;
n += buffer_size(*it);
}
return n;
}
void testBufferCat()
{
using net::buffer_size;
using net::const_buffer;
char buf[10];
std::list<const_buffer> b1;
std::vector<const_buffer> b2{
const_buffer{buf+0, 1},
const_buffer{buf+1, 2}};
std::list<const_buffer> b3;
std::array<const_buffer, 3> b4{{
const_buffer{buf+3, 1},
const_buffer{buf+4, 2},
const_buffer{buf+6, 3}}};
std::list<const_buffer> b5{
const_buffer{buf+9, 1}};
std::list<const_buffer> b6;
auto bs = buffers_cat(
b1, b2, b3, b4, b5, b6);
BEAST_EXPECT(buffer_size(bs) == 10);
BEAST_EXPECT(bsize1(bs) == 10);
BEAST_EXPECT(bsize2(bs) == 10);
BEAST_EXPECT(bsize3(bs) == 10);
BEAST_EXPECT(bsize4(bs) == 10);
std::vector<const_buffer> v;
for(auto iter = make_reverse_iterator(bs.end());
iter != make_reverse_iterator(bs.begin()); ++iter)
v.emplace_back(*iter);
BEAST_EXPECT(buffer_size(bs) == 10);
decltype(bs) bs2(bs);
auto bs3(std::move(bs));
{
net::streambuf sb1, sb2;
BEAST_EXPECT(buffer_size(buffers_cat(
sb1.prepare(5), sb2.prepare(7))) == 12);
sb1.commit(5);
sb2.commit(7);
BEAST_EXPECT(buffer_size(buffers_cat(
sb1.data(), sb2.data())) == 12);
}
for(auto it = bs.begin(); it != bs.end(); ++it)
{
decltype(bs)::const_iterator copy;
copy = it;
BEAST_EXPECT(copy == it);
auto copy2 = copy;
BEAST_EXPECT(copy2 == it);
}
}
void testIterators()
{
using net::buffer_size;
using net::const_buffer;
char buf[9];
std::vector<const_buffer> b1{
const_buffer{buf+0, 1},
const_buffer{buf+1, 2}};
std::array<const_buffer, 3> b2{{
const_buffer{buf+3, 1},
const_buffer{buf+4, 2},
const_buffer{buf+6, 3}}};
auto bs = buffers_cat(b1, b2);
for(int n = 0;
n <= std::distance(bs.begin(), bs.end()); ++n)
{
auto it = std::next(bs.begin(), n);
decltype(it) it2(std::move(it));
it = std::move(it2);
auto pit = &it;
it = std::move(*pit);
}
try
{
std::size_t n = 0;
for(auto it = bs.begin(); n < 100; ++it)
++n;
fail();
}
catch(std::exception const&)
{
pass();
}
// decrement iterator
/* VFALCO
This causes a mysterious "uninitialized variable"
warning related to this function (see comment)
https://code.woboq.org/qt5/include/c++/6.3.1/bits/stl_iterator.h.html#159
*/
#if 0
{
auto const rbegin =
make_reverse_iterator(bs.end());
auto const rend =
make_reverse_iterator(bs.begin());
std::size_t n = 0;
for(auto it = rbegin; it != rend; ++it)
n += buffer_size(*it);
BEAST_EXPECT(n == 9);
}
#endif
try
{
std::size_t n = 0;
for(auto it = bs.end(); n < 100; --it)
++n;
fail();
}
catch(std::exception const&)
{
pass();
}
try
{
buffer_size(*bs.end());
fail();
}
catch(std::exception const&)
{
pass();
}
auto bs2 = bs;
BEAST_EXPECT(bs.begin() != bs2.begin());
BEAST_EXPECT(bs.end() != bs2.end());
decltype(bs)::const_iterator it;
decltype(bs2)::const_iterator it2;
BEAST_EXPECT(it == it2);
// decrement begin() should throw
try
{
--bs.begin();
fail();
}
catch(std::logic_error const&)
{
pass();
}
catch(...)
{
fail();
}
}
void
testDefaultIterators()
{
@@ -275,6 +81,209 @@ public:
}
}
void
testBufferSequence()
{
string_view s = "Hello, world!";
net::const_buffer b1(s.data(), 6);
net::const_buffer b2(
s.data() + b1.size(), s.size() - b1.size());
test_buffer_sequence(
*this, buffers_cat(b1, b2));
}
template<class F>
void
checkException(F&& f)
{
try
{
f();
fail("missing exception", __FILE__, __LINE__);
}
catch(std::logic_error const&)
{
pass();
}
catch(...)
{
fail("wrong exception", __FILE__, __LINE__);
}
}
void
testExceptions()
{
net::const_buffer b1{"He", 2};
net::const_buffer b2{"llo,", 4};
net::const_buffer b3{" world!", 7};
auto const b = beast::buffers_cat(b1, b2, b3);
// Dereferencing a default-constructed iterator
checkException(
[]
{
*(decltype(b)::const_iterator{});
});
// Incrementing a default-constructed iterator
checkException(
[]
{
++(decltype(b)::const_iterator{});
});
// Decrementing a default-constructed iterator
checkException(
[]
{
--(decltype(b)::const_iterator{});
});
// Decrementing an iterator to the beginning
checkException(
[&b]
{
--b.begin();
});
// Dereferencing an iterator to the end
checkException(
[&b]
{
*b.end();
});
// Incrementing an iterator to the end
checkException(
[&b]
{
++b.end();
});
}
void
testEmpty()
{
using net::buffer_size;
struct empty_sequence
{
using value_type = net::const_buffer;
using const_iterator = value_type const*;
const_iterator
begin() const noexcept
{
return &v_;
}
const_iterator
end() const noexcept
{
return begin();
}
private:
value_type v_;
};
{
net::const_buffer b0{};
net::const_buffer b1{"He", 2};
net::const_buffer b2{"llo,", 4};
net::const_buffer b3{" world!", 7};
{
auto const b = beast::buffers_cat(b0, b0);
BEAST_EXPECT(buffer_size(b) == 0);
BEAST_EXPECT(buffers_length(b) == 0);
}
{
auto const b = beast::buffers_cat(b0, b0, b0, b0);
BEAST_EXPECT(buffer_size(b) == 0);
BEAST_EXPECT(buffers_length(b) == 0);
}
{
auto const b = beast::buffers_cat(b1, b2, b3);
BEAST_EXPECT(beast::buffers_to_string(b) == "Hello, world!");
BEAST_EXPECT(buffers_length(b) == 3);
test_buffer_sequence(*this, b);
}
{
auto const b = beast::buffers_cat(b0, b1, b2, b3);
BEAST_EXPECT(beast::buffers_to_string(b) == "Hello, world!");
BEAST_EXPECT(buffers_length(b) == 3);
test_buffer_sequence(*this, b);
}
{
auto const b = beast::buffers_cat(b1, b0, b2, b3);
BEAST_EXPECT(beast::buffers_to_string(b) == "Hello, world!");
BEAST_EXPECT(buffers_length(b) == 3);
test_buffer_sequence(*this, b);
}
{
auto const b = beast::buffers_cat(b1, b2, b0, b3);
BEAST_EXPECT(beast::buffers_to_string(b) == "Hello, world!");
BEAST_EXPECT(buffers_length(b) == 3);
test_buffer_sequence(*this, b);
}
{
auto const b = beast::buffers_cat(b1, b2, b3, b0);
BEAST_EXPECT(beast::buffers_to_string(b) == "Hello, world!");
BEAST_EXPECT(buffers_length(b) == 3);
test_buffer_sequence(*this, b);
}
}
{
auto e1 = net::const_buffer{};
auto b1 = std::array<net::const_buffer, 3>{{
e1,
net::const_buffer{"He", 2},
net::const_buffer{"l", 1} }};
auto b2 = std::array<net::const_buffer, 3>{{
net::const_buffer{"lo", 2},
e1,
net::const_buffer{", ", 2} }};
auto b3 = std::array<net::const_buffer, 3>{{
net::const_buffer{"w", 1},
net::const_buffer{"orld!", 5},
e1 }};
{
auto const b = beast::buffers_cat(
e1, b1, e1, b2, e1, b3, e1);
BEAST_EXPECT(beast::buffers_to_string(b) == "Hello, world!");
BEAST_EXPECT(buffers_length(b) == 6);
}
}
{
auto e1 = net::const_buffer{};
auto e2 = empty_sequence{};
auto b1 = std::array<net::const_buffer, 3>{{
e1,
net::const_buffer{"He", 2},
net::const_buffer{"l", 1} }};
auto b2 = std::array<net::const_buffer, 3>{{
net::const_buffer{"lo", 2},
e1,
net::const_buffer{", ", 2} }};
auto b3 = std::array<net::const_buffer, 3>{
net::const_buffer{"w", 1},
net::const_buffer{"orld!", 5},
e1
};
{
auto const b = beast::buffers_cat(
e2, b1, e2, b2, e2, b3, e2);
BEAST_EXPECT(beast::buffers_to_string(b) == "Hello, world!");
BEAST_EXPECT(buffers_length(b) == 6);
}
}
}
void
testGccWarning1()
{
@@ -307,70 +316,20 @@ public:
char out[64];
const_buffer buffers("Hello, world!", 13);
std::size_t i = 3;
buffers_suffix<const_buffer> cb{buffers};
buffers_suffix<net::const_buffer> cb{buffers};
cb.consume(i);
buffer_copy(
buffer(out),
net::buffer_copy(
net::buffer(out),
buffers_cat(buffers_prefix(i, buffers), cb));
}
void
test_empty_buffer_sequences()
{
using net::buffer_size;
using net::const_buffer;
std::vector<const_buffer> v1;
std::vector<const_buffer> v2;
BEAST_EXPECT(buffer_size(buffers_cat(v1, v2)) == 0);
}
void run() override
{
using net::const_buffer;
using net::const_buffer;
using net::mutable_buffer;
struct user_defined : mutable_buffer
{
};
// Check is_all_const_buffer_sequence
BOOST_STATIC_ASSERT(
detail::is_all_const_buffer_sequence<const_buffer>::value);
BOOST_STATIC_ASSERT(
detail::is_all_const_buffer_sequence<const_buffer, const_buffer>::value);
BOOST_STATIC_ASSERT(
detail::is_all_const_buffer_sequence<mutable_buffer, mutable_buffer>::value);
BOOST_STATIC_ASSERT(
detail::is_all_const_buffer_sequence<const_buffer, mutable_buffer>::value);
BOOST_STATIC_ASSERT(
! detail::is_all_const_buffer_sequence<const_buffer, mutable_buffer, int>::value);
// Ensure that concatenating mutable buffer
// sequences results in a mutable buffer sequence
BOOST_STATIC_ASSERT(std::is_same<
mutable_buffer,
decltype(buffers_cat(
std::declval<mutable_buffer>(),
std::declval<mutable_buffer>(),
std::declval<std::vector<mutable_buffer>>()
))::value_type>::value);
// Ensure that concatenating mixed buffer
// sequences results in a const buffer sequence.
BOOST_STATIC_ASSERT(std::is_same<
const_buffer,
decltype(buffers_cat(
std::declval<mutable_buffer>(),
std::declval<user_defined>(),
std::declval<const_buffer>()
))::value_type>::value);
testBufferCat();
testIterators();
testDefaultIterators();
testBufferSequence();
testExceptions();
testEmpty();
testGccWarning1();
testGccWarning2();
test_empty_buffer_sequences();
}
};