Add emplace_{back,front} to circular_buffer - rebased to latest develop

[f9z]

based on #15 request by AI0867

[f9z]

@AI0867 i've taken your code rebased to develop and made minor adjustments to be compatible with latest changes.
@gpascualg would highly appreciate if you could help me with review please. i used your proposed patch to avoid extra destructor call.

[AI0867]

@f9z Thank you for your efforts. I haven't had the time to set up a build environment on my current machine to do this myself.

[gpascualg]

Looks good to me, plus it's more consistent with the circular_buffer code than the patch I proposed.
Thank you for putting it together.

[f9z]

That'd be the procedure to merge it to "develop" from here @gpascualg ? It's my first PR to boostorg. Anything else I need to do to make it happen? Thanks!

[gpascualg]

I'm afraid I can't really help you here, as I have never done a PR to any boost library. I'd guess, though, that this should be it.

[f9z]

@glenfe would it be possible to consider merging this PR? please let me know if i'd need to do anything else or approach some other boostorg maintainers. thanks!

[lano1106]

what was the problem with the initial replace() code?

I left a comment about what I feel was observed here #15 (comment)

this is an excellent PR but I would sort out the situation and possibly discard the last commit from the PR...

[lano1106]

This has not been compiled with BOOST_NO_CXX11_VARIADIC_TEMPLATES defined because I do not think this would compile.

There is no variadic template parameter Args (args) here

[lano1106]

This has not been compiled with BOOST_NO_CXX11_VARIADIC_TEMPLATES defined because I do not think this would compile.

There is no variadic template parameter Args (args) here

[lano1106]

this line is wrong. This is your bug!

This is where the temporary object gets created.

What do you think value_type(...) is doing?

It creates a temporary object and call the replace() method that is using assignment!

[lano1106]

Here is my take. This is a patch over branch after having reverted "replace replace() with destroy_item() + construct()"

This reverts commit c4c85d7.

diff --git a/include/boost/circular_buffer/base.hpp b/include/boost/circular_buffer/base.hpp
index 5ed1321..846cb32 100644
--- a/include/boost/circular_buffer/base.hpp
+++ b/include/boost/circular_buffer/base.hpp
@@ -1460,7 +1460,7 @@ private:
         if (full()) {
             if (empty())
                 return;
-            replace(m_last, value_type(::boost::forward<Args>(args)...));
+            destructive_replace(m_last, ::boost::forward<Args>(args)...);
             increment(m_last);
             m_first = m_last;
         } else {
@@ -1470,16 +1470,16 @@ private:
         }
     }
 #else
-    template <class V>
-    void emplace_back_impl(BOOST_FWD_REF(V) value) {
+    template <class Arg>
+    void emplace_back_impl(BOOST_FWD_REF(Arg) arg) {
         if (full()) {
             if (empty())
                 return;
-            replace(m_last, value_type(::boost::forward<V>(value)));
+            destructive_replace(m_last, ::boost::forward<Arg>(arg));
             increment(m_last);
             m_first = m_last;
         } else {
-            boost::allocator_construct(alloc(), boost::to_address(m_last), ::boost::forward<V>(value));
+            boost::allocator_construct(alloc(), boost::to_address(m_last), ::boost::forward<Arg>(arg));
             increment(m_last);
             ++m_size;
         }
@@ -1494,7 +1494,7 @@ private:
                 if (empty())
                     return;
                 decrement(m_first);
-                replace(m_first, value_type(::boost::forward<Args>(args)...));
+                destructive_replace(m_first, ::boost::forward<Args>(args)...);
                 m_last = m_first;
             } else {
                 decrement(m_first);
@@ -1508,18 +1508,18 @@ private:
         BOOST_CATCH_END
     }
 #else
-    template <class V>
-    void emplace_front_impl(BOOST_FWD_REF(V) value) {
+    template <class Arg>
+    void emplace_front_impl(BOOST_FWD_REF(Arg) arg) {
         BOOST_TRY {
             if (full()) {
                 if (empty())
                     return;
                 decrement(m_first);
-                replace(m_first, value_type(::boost::forward<V>(value)));
+                destructive_replace(m_first, ::boost::forward<Arg>(arg));
                 m_last = m_first;
             } else {
                 decrement(m_first);
-                boost::allocator_construct(alloc(), boost::to_address(m_first), ::boost::forward<V>(value));
+                boost::allocator_construct(alloc(), boost::to_address(m_first), ::boost::forward<Arg>(arg));
                 ++m_size;
             }
         } BOOST_CATCH(...) {
@@ -2560,6 +2560,30 @@ private:
 #endif
     }
 
+#if !defined(BOOST_NO_CXX11_VARIADIC_TEMPLATES)
+    template<class... Args>
+    void destructive_replace(pointer pos, BOOST_FWD_REF(Args)... args)
+    {
+        boost::allocator_destroy(alloc(), boost::to_address(pos));
+        boost::allocator_construct(alloc(), boost::to_address(pos),
+                                   ::boost::forward<Args>(args)...);
+#if BOOST_CB_ENABLE_DEBUG
+        invalidate_iterators(iterator(this, pos));
+#endif
+    }
+#else
+    template <class Arg>
+    void destructive_replace(pointer pos, BOOST_FWD_REF(Arg) arg)
+    {
+        boost::allocator_destroy(alloc(), boost::to_address(pos));
+        boost::allocator_construct(alloc(), boost::to_address(pos),
+                                   ::boost::forward<Arg>(arg));
+#if BOOST_CB_ENABLE_DEBUG
+        invalidate_iterators(iterator(this, pos));
+#endif
+    }
+#endif
+
     /*! INTERNAL ONLY */
     void construct_or_replace(bool construct, pointer pos, param_value_type item) {
         if (construct)