Fix basic_fields move assignment

Fixes boostorg#2517

include/boost/beast/http/fields.hpp

@@ -204,6 +204,15 @@ class basic_fields
     using alloc_traits =
         beast::detail::allocator_traits<rebind_type>;
 
+    using pocma = typename
+        alloc_traits::propagate_on_container_move_assignment;
+
+    using pocca = typename
+        alloc_traits::propagate_on_container_copy_assignment;
+
+    using pocs = typename
+        alloc_traits::propagate_on_container_swap;
+
     using size_type = typename
         beast::detail::allocator_traits<Allocator>::size_type;
 
@@ -265,7 +274,7 @@ class basic_fields
         as if constructed using the same allocator.
     */
     basic_fields& operator=(basic_fields&&) noexcept(
-        alloc_traits::propagate_on_container_move_assignment::value);
+        pocma::value && std::is_nothrow_move_assignable<Allocator>::value);
 
     /// Copy assignment.
     basic_fields& operator=(basic_fields const&);

include/boost/beast/http/impl/fields.hpp

@@ -23,8 +23,6 @@
 #include <boost/beast/http/chunk_encode.hpp>
 #include <boost/core/exchange.hpp>
 #include <boost/throw_exception.hpp>
-#include <stdexcept>
-#include <string>
 
 namespace boost {
 namespace beast {
@@ -434,15 +432,12 @@ template<class Allocator>
 auto
 basic_fields<Allocator>::
 operator=(basic_fields&& other) noexcept(
-    alloc_traits::propagate_on_container_move_assignment::value)
-      -> basic_fields&
+    pocma::value && std::is_nothrow_move_assignable<Allocator>::value)
+    -> basic_fields&
 {
-    static_assert(is_nothrow_move_assignable<Allocator>::value,
-        "Allocator must be noexcept assignable.");
     if(this == &other)
         return *this;
-    move_assign(other, std::integral_constant<bool,
-        alloc_traits:: propagate_on_container_move_assignment::value>{});
+    move_assign(other, pocma{});
     return *this;
 }
 
@@ -452,8 +447,7 @@ basic_fields<Allocator>::
 operator=(basic_fields const& other) ->
     basic_fields&
 {
-    copy_assign(other, std::integral_constant<bool,
-        alloc_traits::propagate_on_container_copy_assignment::value>{});
+    copy_assign(other, pocca{});
     return *this;
 }
 
@@ -653,8 +647,7 @@ void
 basic_fields<Allocator>::
 swap(basic_fields<Allocator>& other)
 {
-    swap(other, std::integral_constant<bool,
-        alloc_traits::propagate_on_container_swap::value>{});
+    swap(other, pocs{});
 }
 
 template<class Allocator>
@@ -1124,13 +1117,13 @@ basic_fields<Allocator>::
 move_assign(basic_fields& other, std::true_type)
 {
     clear_all();
+    this->get() = std::move(other.get());
     set_ = std::move(other.set_);
     list_ = std::move(other.list_);
     method_ = other.method_;
     target_or_reason_ = other.target_or_reason_;
     other.method_ = {};
     other.target_or_reason_ = {};
-    this->get() = other.get();
 }
 
 template<class Allocator>

test/beast/http/fields.cpp

@@ -28,62 +28,13 @@ class fields_test : public beast::unit_test::suite
     static constexpr std::size_t max_static_buffer =
         sizeof(beast::detail::temporary_buffer);
 
-    template<class T>
-    class test_allocator
-    {
-    public:
-        using value_type = T;
-
-        test_allocator() noexcept(false) {}
-
-        template<class U, class = typename
-            std::enable_if<!std::is_same<test_allocator, U>::value>::type>
-        test_allocator(test_allocator<U> const&) noexcept {}
-
-        value_type*
-        allocate(std::size_t n)
-        {
-            return static_cast<value_type*>(::operator new (n*sizeof(value_type)));
-        }
-
-        void
-        deallocate(value_type* p, std::size_t) noexcept
-        {
-            ::operator delete(p);
-        }
-
-        template<class U>
-        friend
-        bool
-        operator==(test_allocator<T> const&, test_allocator<U> const&) noexcept
-        {
-            return true;
-        }
-
-        template<class U>
-        friend
-        bool
-        operator!=(test_allocator<T> const& x, test_allocator<U> const& y) noexcept
-        {
-            return !(x == y);
-        }
-    };
-
-    using test_fields = basic_fields<test_allocator<char>>;
-
     BOOST_STATIC_ASSERT(is_fields<fields>::value);
-    BOOST_STATIC_ASSERT(is_fields<test_fields>::value);
 
     // std::allocator is noexcept movable, fields should satisfy
     // these constraints as well.
     BOOST_STATIC_ASSERT(std::is_nothrow_move_constructible<fields>::value);
     BOOST_STATIC_ASSERT(std::is_nothrow_move_assignable<fields>::value);
 
-    // Check if basic_fields respects throw-constructibility and
-    // propagate_on_container_move_assignment of the allocator.
-    BOOST_STATIC_ASSERT(std::is_nothrow_move_constructible<test_fields>::value);
-    BOOST_STATIC_ASSERT(!std::is_nothrow_move_assignable<test_fields>::value);
-
     template<class Allocator>
     using fa_t = basic_fields<Allocator>;
 
@@ -1072,6 +1023,72 @@ class fields_test : public beast::unit_test::suite
         BOOST_STATIC_ASSERT(( insert_test<string_view, const char(&)[10]>::value));
     }
 
+    template<class T>
+    class throwing_allocator
+    {
+    public:
+        using value_type = T;
+
+        throwing_allocator() noexcept(false) {}
+
+        throwing_allocator(throwing_allocator const&) noexcept(false) {}
+        throwing_allocator(throwing_allocator&&) noexcept(false) {}
+
+        throwing_allocator& operator=(throwing_allocator const&) noexcept(false) { return *this; }
+        throwing_allocator& operator=(throwing_allocator&&) noexcept(false) { return *this; }
+
+        template<class U, class = typename
+            std::enable_if<!std::is_same<throwing_allocator, U>::value>::type>
+        throwing_allocator(throwing_allocator<U> const&) noexcept(false) {}
+
+        value_type*
+        allocate(std::size_t n)
+        {
+            return static_cast<value_type*>(::operator new (n*sizeof(value_type)));
+        }
+
+        void
+        deallocate(value_type* p, std::size_t) noexcept
+        {
+            ::operator delete(p);
+        }
+
+        template<class U>
+        friend
+        bool
+        operator==(throwing_allocator<T> const&, throwing_allocator<U> const&) noexcept
+        {
+            return true;
+        }
+
+        template<class U>
+        friend
+        bool
+        operator!=(throwing_allocator<T> const& x, throwing_allocator<U> const& y) noexcept
+        {
+            return !(x == y);
+        }
+    };
+
+    void
+    testIssue2517()
+    {
+        using test_fields = basic_fields<throwing_allocator<char>>;
+        BOOST_STATIC_ASSERT(is_fields<test_fields>::value);
+
+        // Check if basic_fields respects throw-constructibility and
+        // propagate_on_container_move_assignment of the allocator.
+        BOOST_STATIC_ASSERT(std::is_nothrow_move_constructible<test_fields>::value);
+        BOOST_STATIC_ASSERT(!std::is_nothrow_move_assignable<test_fields>::value);
+
+        test_fields f1;
+        f1.insert("1", "1");
+        test_fields f2;
+        f2 = std::move(f1);
+        BEAST_EXPECT(f1.begin() == f1.end());
+        BEAST_EXPECT(f2["1"] == "1");
+    }
+
     void
     testEmpty()
     {
@@ -1103,6 +1120,7 @@ class fields_test : public beast::unit_test::suite
 
         testIssue1828();
         boost::ignore_unused(&fields_test::testIssue2085);
+        testIssue2517();
         testEmpty();
     }
 };