Allowing for non-default-constructible action argument and action ret…

…urn types
STEllAR-GROUP · Aug 21, 2017 · 82835bd · 82835bd
1 parent 7d28e21
commit 82835bd
Show file tree

Hide file tree

Showing 8 changed files with 245 additions and 13 deletions.
diff --git a/hpx/lcos/base_lco_with_value.hpp b/hpx/lcos/base_lco_with_value.hpp
@@ -17,6 +17,7 @@
 #include <hpx/runtime/components_fwd.hpp>
 #include <hpx/runtime/naming/id_type.hpp>
 #include <hpx/traits/is_component.hpp>
+#include <hpx/util/assert.hpp>
 #include <hpx/util/detail/pp/cat.hpp>
 #include <hpx/util/detail/pp/expand.hpp>
 #include <hpx/util/detail/pp/nargs.hpp>
@@ -81,6 +82,17 @@ namespace hpx { namespace lcos
         virtual ~base_lco_with_value() noexcept {}
 
         virtual void set_event()
+        {
+            set_event_nonvirt(std::is_default_constructible<RemoteResult>());
+        }
+
+        void set_event_nonvirt(std::false_type)
+        {
+            // this shouldn't ever be called
+            HPX_ASSERT(false);
+        }
+
+        void set_event_nonvirt(std::true_type)
         {
             set_value(RemoteResult());
         }

diff --git a/hpx/runtime/actions/transfer_action.hpp b/hpx/runtime/actions/transfer_action.hpp
@@ -213,13 +213,13 @@ namespace hpx { namespace actions
 
         if (deferred_schedule)
         {
-            // If this is a direct action and deferred schedule was requested, that
-            // is we are not the last parcel, return immediately
+            // If this is a direct action and deferred schedule was requested,
+            // that is we are not the last parcel, return immediately
             if (base_type::direct_execution::value)
                 return;
 
-            // If this is not a direct action, we can safely set deferred_schedule
-            // to false
+            // If this is not a direct action, we can safely set
+            // deferred_schedule to false
             deferred_schedule = false;
         }
 

diff --git a/hpx/runtime/actions/transfer_base_action.hpp b/hpx/runtime/actions/transfer_base_action.hpp
@@ -19,24 +19,105 @@
 #include <hpx/runtime/serialization/base_object.hpp>
 #include <hpx/runtime/serialization/input_archive.hpp>
 #include <hpx/runtime/serialization/output_archive.hpp>
+#include <hpx/runtime/serialization/unique_ptr.hpp>
 #include <hpx/traits/action_does_termination_detection.hpp>
 #include <hpx/traits/action_message_handler.hpp>
 #include <hpx/traits/action_was_object_migrated.hpp>
 #include <hpx/traits/action_priority.hpp>
 #include <hpx/traits/action_schedule_thread.hpp>
 #include <hpx/traits/action_serialization_filter.hpp>
 #include <hpx/traits/action_stacksize.hpp>
+#include <hpx/util/assert.hpp>
 #include <hpx/util/get_and_reset_value.hpp>
 #include <hpx/util/serialize_exception.hpp>
+#include <hpx/util/tuple.hpp>
 
 #include <boost/atomic.hpp>
 
 #include <cstddef>
 #include <cstdint>
+#include <memory>
+#include <type_traits>
 #include <utility>
 
 namespace hpx { namespace actions
 {
+    ///////////////////////////////////////////////////////////////////////////
+    // If one or more arguments of the action are non-default-constructible,
+    // the transfer_action does not store the argument tuple directly but a
+    // unique_ptr to the tuple instead.
+    namespace detail
+    {
+        template <typename Args>
+        struct argument_holder
+        {
+            argument_holder() = default;
+
+            explicit argument_holder(Args && args)
+              : data_(new Args(std::move(args)))
+            {}
+
+            template <typename ... Ts>
+            argument_holder(Ts && ... ts)
+              : data_(new Args(std::forward<Ts>(ts)...))
+            {}
+
+            template <typename Archive>
+            void serialize(Archive& ar, unsigned int const)
+            {
+                ar & data_;
+            }
+
+            std::unique_ptr<Args> data_;
+        };
+    }
+}}
+
+namespace hpx { namespace util
+{
+    template <std::size_t I, typename Args>
+    HPX_CONSTEXPR HPX_HOST_DEVICE HPX_FORCEINLINE
+    typename util::tuple_element<I, Args>::type&
+    get(hpx::actions::detail::argument_holder<Args>& t) noexcept
+    {
+        HPX_ASSERT(!!t.data_);
+        return util::tuple_element<I, Args>::get(*t.data_);
+    }
+
+    template <std::size_t I, typename Args>
+    HPX_CONSTEXPR HPX_HOST_DEVICE HPX_FORCEINLINE
+    typename util::tuple_element<I, Args>::type const&
+    get(hpx::actions::detail::argument_holder<Args> const& t) noexcept
+    {
+        HPX_ASSERT(!!t.data_);
+        return util::tuple_element<I, Args>::get(*t.data_);
+    }
+
+    template <std::size_t I, typename Args>
+    HPX_CONSTEXPR HPX_HOST_DEVICE HPX_FORCEINLINE
+    typename util::tuple_element<I, Args>::type&&
+    get(hpx::actions::detail::argument_holder<Args>&& t) noexcept
+    {
+        HPX_ASSERT(!!t.data_);
+        return std::forward<typename util::tuple_element<I, Args>::type>(
+            util::get<I>(*t.data_));
+    }
+
+    template <std::size_t I, typename Args>
+    HPX_CONSTEXPR HPX_HOST_DEVICE HPX_FORCEINLINE
+    typename util::tuple_element<I, Args>::type const&&
+    get(hpx::actions::detail::argument_holder<Args> const&& t) noexcept
+    {
+        HPX_ASSERT(!!t.data_);
+        return std::forward<
+                typename util::tuple_element<I, Args>::type const
+            >(util::get<I>(*t.data_));
+    }
+}}
+
+namespace hpx { namespace actions
+{
+    ///////////////////////////////////////////////////////////////////////////
     template <typename Action>
     struct transfer_base_action : base_action
     {
@@ -47,7 +128,12 @@ namespace hpx { namespace actions
         typedef typename Action::component_type component_type;
         typedef typename Action::derived_type derived_type;
         typedef typename Action::result_type result_type;
-        typedef typename Action::arguments_type arguments_type;
+        typedef typename Action::arguments_type arguments_base_type;
+        typedef typename std::conditional<
+                std::is_constructible<arguments_base_type>::value,
+                    arguments_base_type,
+                    detail::argument_holder<arguments_base_type>
+            >::type arguments_type;
         typedef typename Action::continuation_type continuation_type;
 
         // This is the priority value this action has been instantiated with

diff --git a/hpx/runtime/actions/transfer_continuation_action.hpp b/hpx/runtime/actions/transfer_continuation_action.hpp
@@ -37,6 +37,7 @@ namespace hpx { namespace actions
 
         typedef transfer_base_action<Action> base_type;
         typedef typename base_type::continuation_type continuation_type;
+
     public:
         // construct an empty transfer_continuation_action to avoid serialization
         // overhead

diff --git a/hpx/runtime/serialization/detail/polymorphic_nonintrusive_factory.hpp b/hpx/runtime/serialization/detail/polymorphic_nonintrusive_factory.hpp
@@ -32,19 +32,17 @@
 namespace hpx { namespace serialization { namespace detail
 {
     ///////////////////////////////////////////////////////////////////////////
-    // default fall-backs for constructing non-default-constructable types
-    template <class Archive, class T>
-    void save_construct_data(Archive&, T*, unsigned)
+    // default fall-backs for constructing non-default-constructible types
+    template <typename Archive, typename T>
+    void save_construct_data(Archive&, T const*, unsigned)
     {
-        // a user is not required to provide their own adl-overload
+        // users are not required to provide their own adl-overload
     }
 
-    template <class Archive, class T>
+    template <typename Archive, typename T>
     void load_construct_data(Archive& ar, T* t, unsigned)
     {
-        // this function is never supposed to be called
-        HPX_ASSERT(false);
-        ::new (t) T;
+        ::new (t) T;  // by default fall back to in-place default construction
     }
 
     ///////////////////////////////////////////////////////////////////////////

diff --git a/hpx/util/tuple.hpp b/hpx/util/tuple.hpp
@@ -68,6 +68,10 @@ namespace hpx { namespace util
         struct tuple_member //-V690
         {
         public:
+            template <typename U = T,
+                typename Enable = typename std::enable_if<
+                    std::is_constructible<U>::value
+                >::type>
             HPX_CONSTEXPR HPX_HOST_DEVICE tuple_member()
               : _value()
             {}
@@ -206,6 +210,13 @@ namespace hpx { namespace util
           : tuple_member<Is, Ts>...
         {
             // 20.4.2.1, tuple construction
+            template <typename Dependent = void,
+                typename Enable = typename std::enable_if<
+                    hpx::util::detail::all_of<
+                        std::is_constructible<Ts>...
+                    >::value,
+                    Dependent
+                >::type>
             HPX_CONSTEXPR HPX_HOST_DEVICE tuple_impl()
               : tuple_member<Is, Ts>()...
             {}
@@ -304,6 +315,26 @@ namespace hpx { namespace util
                 };
                 (void)_sequencer;
             }
+
+            template <typename Archive>
+            friend void load_construct_data(
+                Archive& ar, tuple_impl* t, unsigned int const version)
+            {
+                int const _sequencer[] = {
+                    (load_construct_data(ar, &t->get<Is>(), version), 0)...
+                };
+                (void)_sequencer;
+            }
+
+            template <typename Archive>
+            friend void save_construct_data(
+                Archive& ar, tuple_impl const* t, unsigned int const version)
+            {
+                int const _sequencer[] = {
+                    (save_construct_data(ar, &t->get<Is>(), version), 0)...
+                };
+                (void)_sequencer;
+            }
         };
 
         ///////////////////////////////////////////////////////////////////////
@@ -377,6 +408,11 @@ namespace hpx { namespace util
 
         // constexpr tuple();
         // Value initializes each element.
+        template <typename Dependent = void,
+            typename Enable = typename std::enable_if<
+               hpx::util::detail::all_of<std::is_constructible<Ts>...>::value,
+               Dependent
+            >::type>
         HPX_CONSTEXPR HPX_HOST_DEVICE tuple()
           : _impl()
         {}
@@ -1060,6 +1096,26 @@ namespace hpx { namespace serialization
       , unsigned int const version = 0
     )
     {}
+
+    template <typename Archive, typename ...Ts>
+    HPX_FORCEINLINE
+    void load_construct_data(
+        Archive& ar
+      , ::hpx::util::tuple<Ts...>* t
+      , unsigned int const version = 0)
+    {
+        load_construct_data(ar, &(t->_impl), version);
+    }
+
+    template <typename Archive, typename ...Ts>
+    HPX_FORCEINLINE
+    void save_construct_data(
+        Archive& ar
+      , ::hpx::util::tuple<Ts...> const* t
+      , unsigned int const version = 0)
+    {
+        save_construct_data(ar, &(t->_impl), version);
+    }
 }}
 
 #if defined(HPX_MSVC_WARNING_PRAGMA)

diff --git a/tests/regressions/actions/CMakeLists.txt b/tests/regressions/actions/CMakeLists.txt
@@ -13,6 +13,7 @@ set(tests
     plain_action_1550
     plain_action_move_semantics
     return_future_2847
+    return_non_default_constructible_2847
    )
 
 set(plain_action_1330_FLAGS
@@ -31,6 +32,7 @@ set(component_action_move_semantics_PARAMETERS
     THREADS_PER_LOCALITY 1)
 
 set(return_future_2847_PARAMETERS LOCALITIES 2)
+set(return_non_default_constructible_2847_PARAMETERS LOCALITIES 2)
 
 foreach(test ${tests})
   set(sources

diff --git a/tests/regressions/actions/return_non_default_constructible_2847.cpp b/tests/regressions/actions/return_non_default_constructible_2847.cpp
@@ -0,0 +1,77 @@
+//  Copyright (c) 2014-2017 Hartmut Kaiser
+//  Copyright (c) 2017 Igor Krivenko
+//
+//  Distributed under the Boost Software License, Version 1.0. (See accompanying
+//  file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
+
+#include <hpx/hpx_main.hpp>
+#include <hpx/include/actions.hpp>
+#include <hpx/include/components.hpp>
+#include <hpx/include/lcos.hpp>
+#include <hpx/util/lightweight_test.hpp>
+
+#include <cstddef>
+#include <vector>
+
+struct non_default_ctor
+{
+    int i;
+
+    non_default_ctor() = delete;
+    non_default_ctor(int i)
+      : i(i)
+    {
+    }
+
+    template <typename Archive>
+    void serialize(Archive& ar, const unsigned int)
+    {
+        ar & i;
+    }
+
+    template <typename Archive>
+    void friend load_construct_data(
+        Archive& ar, non_default_ctor* p, const unsigned int)
+    {
+        ::new (p) non_default_ctor(0);
+    }
+};
+
+non_default_ctor plain_non_default_ctor()
+{
+    return non_default_ctor(42);
+}
+
+HPX_PLAIN_ACTION(plain_non_default_ctor, plain_non_default_ctor_action);
+
+void test_plain_call_non_default_ctor(hpx::id_type id)
+{
+    // test apply
+    for (std::size_t i = 0; i != 100; ++i)
+    {
+        hpx::apply<plain_non_default_ctor_action>(id);
+    }
+
+    // test async
+    std::vector<hpx::future<non_default_ctor>> calls;
+    for (std::size_t i = 0; i != 100; ++i)
+    {
+        calls.push_back(hpx::async<plain_non_default_ctor_action>(id));
+    }
+    hpx::wait_all(calls);
+
+    for (auto && f : calls)
+    {
+        HPX_TEST_EQ(f.get().i, 42);
+    }
+}
+
+///////////////////////////////////////////////////////////////////////////////
+int main()
+{
+    for (hpx::id_type id : hpx::find_all_localities())
+    {
+        test_plain_call_non_default_ctor(id);
+    }
+    return hpx::util::report_errors();
+}