Refactor buffers_suffix:

* Tidy up tests
* Increase code coverage
* Remove unnecessary move special members
* Correct behavior for default constructed iterators

CHANGELOG.md

@@ -11,6 +11,7 @@ Version 200
 * Add buffers_iterator_type trait
 * Use new buffer traits, remove old unused traits
 * Optimize for size on buffers_cat preconditions
+* Refactor buffers_suffix
 
 API Changes:
 

include/boost/beast/core/buffers_suffix.hpp

@@ -78,7 +78,7 @@ public:
         `net::const_buffer`.
     */
 #if BOOST_BEAST_DOXYGEN
-    using value_type = __implementation_defined__;
+    using value_type = __see_below__;
 #elif 0
     using value_type = typename std::conditional<
         std::is_convertible<typename
@@ -121,6 +121,7 @@ public:
         @param args Arguments forwarded to the buffers constructor.
     */
     template<class... Args>
+    explicit
     buffers_suffix(boost::in_place_init_t, Args&&... args);
 
     /// Assignment

include/boost/beast/core/impl/buffers_suffix.hpp

@@ -49,9 +49,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
@@ -69,9 +67,9 @@ public:
     reference
     operator*() const
     {
-        return it_ == b_->begin_
+        if(it_ == b_->begin_)
-            ? value_type{*it_} + b_->skip_
+            return value_type(*it_) + b_->skip_;
-            : *it_;
+        return value_type{*it_};
     }
 
     pointer

test/beast/core/buffer_test.hpp

@@ -52,6 +52,7 @@ public:
 
     buffers_triple(
         buffers_triple const&) = default;
+
     buffers_triple& operator=(
         buffers_triple const&) = default;
 

test/beast/core/buffers_suffix.cpp

@@ -13,7 +13,6 @@
 #include "buffer_test.hpp"
 
 #include <boost/beast/core/buffers_cat.hpp>
-#include <boost/beast/core/ostream.hpp>
 #include <boost/beast/_experimental/unit_test/suite.hpp>
 #include <boost/asio/buffer.hpp>
 #include <string>
@@ -24,6 +23,74 @@ namespace beast {
 class buffers_suffix_test : public beast::unit_test::suite
 {
 public:
+    void
+    testBufferSequence()
+    {
+        // mutable
+        {
+            char buf[13];
+            auto b = buffers_triple(buf, sizeof(buf));
+            buffers_suffix<decltype(b)> bs(b);
+            test_buffer_sequence(*this, bs);
+        }
+
+        // const
+        {
+            string_view src = "Hello, world!";
+            std::array<net::const_buffer, 3> b{{
+                net::const_buffer(src.data(),     3),
+                net::const_buffer(src.data() + 3, 4),
+                net::const_buffer(src.data() + 7, 6) }};
+            buffers_suffix<decltype(b)> bs(b);
+            test_buffer_sequence(*this, bs);
+        }
+    }
+
+    void
+    testSpecial()
+    {
+        // default construction
+        {
+            class test_buffer
+                : public net::const_buffer
+            {
+            public:
+                test_buffer()
+                    : net::const_buffer("\r\n", 2)
+                {
+                }
+            };
+
+            buffers_suffix<test_buffer> cb;
+            BEAST_EXPECT(buffers_to_string(cb) == "\r\n");
+            cb.consume(1);
+            BEAST_EXPECT(buffers_to_string(cb) == "\n");
+        }
+
+        // in-place init
+        {
+            using namespace test;
+            buffers_suffix<buffers_cat_view<
+                net::const_buffer,
+                net::const_buffer>> cb(
+                    boost::in_place_init,
+                        net::const_buffer("\r", 1),
+                        net::const_buffer("\n", 1));
+            BEAST_EXPECT(buffers_to_string(cb) == "\r\n");
+        }
+
+        // empty sequence
+        {
+            using net::buffer_size;
+            buffers_suffix<net::mutable_buffer> cb(
+                net::mutable_buffer{});
+            BEAST_EXPECT(buffer_size(cb) == 0);
+            buffers_suffix<net::mutable_buffer> cb2(
+                net::mutable_buffer{});
+            BEAST_EXPECT(net::buffer_copy(cb2, cb) == 0);
+        }
+    }
+
     template<class BufferSequence>
     static
     buffers_suffix<BufferSequence>
@@ -43,41 +110,15 @@ public:
         return buffers_to_string(lhs) == buffers_to_string(rhs);
     }
 
-    template<class ConstBufferSequence>
-    void
-    expect_size(std::size_t n, ConstBufferSequence const& buffers)
-    {
-        BEAST_EXPECT(test::size_pre(buffers) == n);
-        BEAST_EXPECT(test::size_post(buffers) == n);
-        BEAST_EXPECT(test::size_rev_pre(buffers) == n);
-        BEAST_EXPECT(test::size_rev_post(buffers) == n);
-    }
-
-    void
-    testMembers()
-    {
-        char buf[12];
-        buffers_suffix<
-            net::const_buffer> cb1{
-                boost::in_place_init, buf, sizeof(buf)};
-        buffers_suffix<
-            net::const_buffer> cb2{
-                boost::in_place_init, nullptr, 0};
-        cb2 = cb1;
-        cb1 = std::move(cb2);
-    }
-
     void
     testMatrix()
     {
-        using namespace test;
+        using net::buffer_size;
-        using net::buffer;
-        using net::const_buffer;
         char buf[12];
         std::string const s = "Hello, world";
         BEAST_EXPECT(s.size() == sizeof(buf));
-        buffer_copy(buffer(buf), buffer(s));
+        net::buffer_copy(net::buffer(buf), net::buffer(s));
-        BEAST_EXPECT(buffers_to_string(buffer(buf)) == s);
+        BEAST_EXPECT(buffers_to_string(net::buffer(buf)) == s);
         for(std::size_t i = 1; i < 4; ++i) {
         for(std::size_t j = 1; j < 4; ++j) {
         for(std::size_t x = 1; x < 4; ++x) {
@@ -85,17 +126,17 @@ public:
         std::size_t k = sizeof(buf) - (i + j);
         std::size_t z = sizeof(buf) - (x + y);
         {
-            std::array<const_buffer, 3> bs{{
+            std::array<net::const_buffer, 3> bs{{
-                const_buffer{&buf[0], i},
+                net::const_buffer{&buf[0], i},
-                const_buffer{&buf[i], j},
+                net::const_buffer{&buf[i], j},
-                const_buffer{&buf[i+j], k}}};
+                net::const_buffer{&buf[i+j], k}}};
             buffers_suffix<decltype(bs)> cb(bs);
             BEAST_EXPECT(buffers_to_string(cb) == s);
-            expect_size(s.size(), cb);
+            BEAST_EXPECT(buffer_size(cb) == s.size());
             cb.consume(0);
             BEAST_EXPECT(eq(cb, consumed_buffers(bs, 0)));
             BEAST_EXPECT(buffers_to_string(cb) == s);
-            expect_size(s.size(), cb);
+            BEAST_EXPECT(buffer_size(cb) == s.size());
             cb.consume(x);
             BEAST_EXPECT(buffers_to_string(cb) == s.substr(x));
             BEAST_EXPECT(eq(cb, consumed_buffers(bs, x)));
@@ -113,69 +154,11 @@ public:
     }
 
     void
-    testDefaultCtor()
+    run() override
     {
-        using namespace test;
+        testBufferSequence();
-        class test_buffer : public net::const_buffer
+        testSpecial();
-        {
-        public:
-            test_buffer()
-                : net::const_buffer("\r\n", 2)
-            {
-            }
-        };
-
-        buffers_suffix<test_buffer> cb;
-        BEAST_EXPECT(buffers_to_string(cb) == "\r\n");
-    }
-
-    void
-    testInPlace()
-    {
-        using namespace test;
-        buffers_suffix<buffers_cat_view<
-            net::const_buffer,
-            net::const_buffer>> cb(
-                boost::in_place_init,
-                    net::const_buffer("\r", 1),
-                    net::const_buffer("\n", 1));
-        BEAST_EXPECT(buffers_to_string(cb) == "\r\n");
-    }
-
-    void
-    testEmptyBuffers()
-    {
-        using net::buffer_copy;
-        using net::buffer_size;
-        using net::mutable_buffer;
-        buffers_suffix<mutable_buffer> cb(
-            mutable_buffer{});
-        BEAST_EXPECT(buffer_size(cb) == 0);
-        buffers_suffix<mutable_buffer> cb2(
-            mutable_buffer{});
-        BEAST_EXPECT(buffer_copy(cb2, cb) == 0);
-    }
-
-    void
-    testIterator()
-    {
-        using net::const_buffer;
-        std::array<const_buffer, 3> ba;
-        buffers_suffix<decltype(ba)> cb(ba);
-        std::size_t n = 0;
-        for(auto it = cb.end(); it != cb.begin(); --it)
-            ++n;
-        BEAST_EXPECT(n == 3);
-    }
-
-    void run() override
-    {
-        testMembers();
         testMatrix();
-        testDefaultCtor();
-        testInPlace();
-        testEmptyBuffers();
-        testIterator();
     }
 };