Fix basic_fields move assignment

Fixes boostorg#2517
ashtum · Jun 8, 2024 · 826c6b2 · 826c6b2
1 parent b5d7c99
commit 826c6b2
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 14 deletions.
diff --git a/include/boost/beast/http/fields.hpp b/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{} && std::is_nothrow_move_assignable<Allocator>{});
 
     /// Copy assignment.
     basic_fields& operator=(basic_fields const&);

diff --git a/include/boost/beast/http/impl/fields.hpp b/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)
+        pocma{} && std::is_nothrow_move_assignable<Allocator>{})
       -> 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>

diff --git a/test/beast/http/fields.cpp b/test/beast/http/fields.cpp
@@ -34,12 +34,18 @@ class fields_test : public beast::unit_test::suite
     public:
         using value_type = T;
 
-        test_allocator() noexcept(false) {}
+        test_allocator() noexcept(false) = default;
 
         template<class U, class = typename
             std::enable_if<!std::is_same<test_allocator, U>::value>::type>
         test_allocator(test_allocator<U> const&) noexcept {}
 
+        test_allocator(test_allocator const&) = default;
+        test_allocator(test_allocator&&) = default;
+
+        test_allocator& operator=(test_allocator const&) noexcept(false) = default;
+        test_allocator& operator=(test_allocator&&) noexcept(false)  = default;
+
         value_type*
         allocate(std::size_t n)
         {
@@ -1072,6 +1078,17 @@ class fields_test : public beast::unit_test::suite
         BOOST_STATIC_ASSERT(( insert_test<string_view, const char(&)[10]>::value));
     }
 
+    void
+    testIssue2517()
+    {
+        basic_fields<test_allocator<char>> f1;
+        f1.insert("1", "1");
+        basic_fields<test_allocator<char>> f2;
+        f2 = std::move(f1);
+        BEAST_EXPECT(f1.begin() == f1.end());
+        BEAST_EXPECT(f2["1"] == "1");
+    }
+
     void
     testEmpty()
     {