remove copying when invoking objects stored in sbo

Resolves: #23

Right now the invoke holders take the storage by-value which is strictly
incorrect when using SBO as it creates a copy of the callable which
leads to surprising results in user-code that expect multiple calls
to a lambda with a mutable capture.

Signed-off-by: Christian Mazakas <christian.mazakas@gmail.com>

Author: cmazakas

include/boost/compat/move_only_function.hpp

@@ -247,7 +247,7 @@ bool is_nullary_arg( F f )
 template<bool NoEx, class R, class ...Args>
 struct mo_invoke_function_holder
 {
-    static R invoke_function( storage s, Args&&... args) noexcept( NoEx )
+    static R invoke_function( storage const& s, Args&&... args) noexcept( NoEx )
     {
         auto f = reinterpret_cast<R(*)( Args... )>( s.pfn_ );
         return compat::invoke_r<R>( f, std::forward<Args>( args )... );
@@ -257,7 +257,7 @@ struct mo_invoke_function_holder
 template<ref_quals RQ, bool Const, bool NoEx, class F, class R, class ...Args>
 struct mo_invoke_object_holder
 {
-    static R invoke_object( storage s, Args&&... args ) noexcept( NoEx )
+    static R invoke_object( storage const& s, Args&&... args ) noexcept( NoEx )
     {
         using T = remove_reference_t<F>;
         using cv_T = conditional_t<Const, add_const_t<T>, T>;
@@ -274,7 +274,7 @@ struct mo_invoke_object_holder
 template<ref_quals RQ, bool Const, bool NoEx, class F, class R, class ...Args>
 struct mo_invoke_local_holder
 {
-    static R invoke_local( storage s, Args&&... args ) noexcept( NoEx )
+    static R invoke_local( storage const& s, Args&&... args ) noexcept( NoEx )
     {
         using T = remove_reference_t<F>;
         using cv_T = conditional_t<Const, add_const_t<T>, T>;
@@ -285,7 +285,7 @@ struct mo_invoke_local_holder
             >
         >;
 
-        return compat::invoke_r<R>( static_cast<cv_ref_T>( *static_cast<cv_T*>( s.addr() ) ), std::forward<Args>( args )... );
+        return compat::invoke_r<R>( static_cast<cv_ref_T>( *static_cast<cv_T*>( const_cast<storage&>( s ).addr() ) ), std::forward<Args>( args )... );
     }
 };
 
@@ -485,9 +485,9 @@ struct move_only_function_base
 
     detail::storage s_;
 #if defined(__cpp_noexcept_function_type)
-    R ( *invoke_ )( detail::storage, Args&&... ) noexcept( NoEx ) = nullptr;
+    R ( *invoke_ )( detail::storage const&, Args&&... ) noexcept( NoEx ) = nullptr;
 #else
-    R ( *invoke_ )( detail::storage, Args&&... ) = nullptr;
+    R ( *invoke_ )( detail::storage const&, Args&&... ) = nullptr;
 #endif
     void ( *manager_ )( op_type, detail::storage&, detail::storage* ) = &manage_empty;
 };

test/move_only_function_test.cpp

@@ -13,7 +13,8 @@
 #include <boost/core/lightweight_test.hpp>
 #include <boost/core/lightweight_test_trait.hpp>
 
-#include <functional>
+#include <array>
+#include <cstdint>
 #include <memory>
 #include <type_traits>
 
@@ -25,6 +26,11 @@
 #   pragma clang diagnostic ignored "-Wself-move"
 #endif
 
+#ifdef _MSC_VER
+#pragma warning(disable: 4789) // false buffer overrun warning in test_mutable_lambda()
+#endif
+
+
 using std::is_same;
 using std::is_constructible;
 using std::is_nothrow_constructible;
@@ -845,11 +851,39 @@ static void test_conv()
     }
 }
 
+static void test_mutable_lambda()
+{
+    {
+        // Within SBO limits.
+        int captured = 0;
+        move_only_function<int()> func = [captured]() mutable { return ++captured; };
+
+        BOOST_TEST_EQ( func(), 1 );
+        BOOST_TEST_EQ( func(), 2 );
+
+        move_only_function<int()> func2(std::move(func));
+        BOOST_TEST_EQ( func2(), 3 );
+    }
+
+    {
+        // Too large for SBO.
+        std::array<std::uint8_t, 256> captured = {{}};
+        move_only_function<int()> func = [captured]() mutable { return ++captured[0]; };
+
+        BOOST_TEST_EQ( func(), 1 );
+        BOOST_TEST_EQ( func(), 2 );
+
+        move_only_function<int()> func2(std::move(func));
+        BOOST_TEST_EQ( func2(), 3 );
+    }
+}
+
 int main()
 {
     test_call();
     test_traits();
     test_conv();
+    test_mutable_lambda();
 
     return boost::report_errors();
 }