Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allowing for non-default-constructible component types #2851

Merged
merged 1 commit into from Aug 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 19 additions & 1 deletion hpx/runtime/components/component_factory.hpp
Expand Up @@ -49,6 +49,7 @@
#include <hpx/runtime/naming/name.hpp>
#include <hpx/runtime/naming/resolver_client.hpp>
#include <hpx/throw_exception.hpp>
#include <hpx/util/assert.hpp>
#include <hpx/util/atomic_count.hpp>
#include <hpx/util/detail/pp/cat.hpp>
#include <hpx/util/detail/pp/expand.hpp>
Expand All @@ -58,6 +59,7 @@

#include <cstddef>
#include <string>
#include <type_traits>

///////////////////////////////////////////////////////////////////////////////
namespace hpx { namespace components
Expand Down Expand Up @@ -165,10 +167,26 @@ namespace hpx { namespace components
/// created (\a count > 1) the GID's of all new instances are
/// sequential in a row.
naming::gid_type create(std::size_t count = 1)
{
return create<Component>(count,
std::is_default_constructible<
typename Component::type_holder>());
}

template <typename Component_>
naming::gid_type create(std::size_t count, std::false_type)
{
// shouldn't ever be called for non-default-constructible types
HPX_ASSERT(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have to resort to a runtime error here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you suggest? A static assert would trigger for any non-default constructible type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only if we call new_ without arguments, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, always - this function is called by create(size_t) which is a virtual function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, right, too bad :/

return naming::invalid_gid;
}

template <typename Component_>
naming::gid_type create(std::size_t count, std::true_type)
{
if (isenabled_)
{
naming::gid_type id = server::create<Component>(count);
naming::gid_type id = server::create<Component_>(count);
if (id)
++refcnt_;
return id;
Expand Down
18 changes: 18 additions & 0 deletions hpx/runtime/components/derived_component_factory.hpp
Expand Up @@ -15,6 +15,7 @@
#include <hpx/runtime/components/unique_component_name.hpp>
#include <hpx/runtime/naming/resolver_client.hpp>
#include <hpx/throw_exception.hpp>
#include <hpx/util/assert.hpp>
#include <hpx/util/atomic_count.hpp>
#include <hpx/util/detail/pp/cat.hpp>
#include <hpx/util/detail/pp/expand.hpp>
Expand All @@ -24,6 +25,7 @@

#include <cstddef>
#include <string>
#include <type_traits>

///////////////////////////////////////////////////////////////////////////////
namespace hpx { namespace components
Expand Down Expand Up @@ -152,6 +154,22 @@ namespace hpx { namespace components
/// created (\a count > 1) the GID's of all new instances are
/// sequential in a row.
naming::gid_type create(std::size_t count = 1)
{
return create<Component>(count,
std::is_default_constructible<
typename Component::type_holder>());
}

template <typename Component_>
naming::gid_type create(std::size_t count, std::false_type)
{
// shouldn't ever be called for non-default-constructible types
HPX_ASSERT(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

return naming::invalid_gid;
}

template <typename Component_>
naming::gid_type create(std::size_t count, std::true_type)
{
if (isenabled_)
{
Expand Down
6 changes: 5 additions & 1 deletion hpx/runtime/components/server/managed_component_base.hpp
Expand Up @@ -516,7 +516,11 @@ namespace hpx { namespace components

/// \brief The function \a create is used for allocation and
// initialization of arrays of wrappers.
static value_type* create(std::size_t count = 1)
template <typename T = Component>
static typename std::enable_if<
std::is_default_constructible<T>::value, value_type*
>::type
create(std::size_t count = 1)
{
// allocate the memory
void* p = heap_type::alloc(count);
Expand Down
6 changes: 5 additions & 1 deletion hpx/runtime/components/server/simple_component_base.hpp
Expand Up @@ -297,7 +297,11 @@ namespace hpx { namespace components

/// \brief The function \a create is used for allocation and
/// initialization of instances of the derived components.
static component_type* create(std::size_t count)
template <typename T = Component>
static typename std::enable_if<
std::is_default_constructible<T>::value, component_type*
>::type
create(std::size_t count)
{
// simple components can be created individually only
HPX_ASSERT(1 == count);
Expand Down
2 changes: 2 additions & 0 deletions tests/regressions/components/CMakeLists.txt
Expand Up @@ -7,11 +7,13 @@ set(tests
create_n_components_2323
create_remote_component_2334
moveonly_constructor_arguments_1405
new_2848
partitioned_vector_2201
returned_client_2150
)

set(create_remote_component_2334_PARAMETERS LOCALITIES 2)
set(new_2848_PARAMETERS LOCALITIES 2)

foreach(test ${tests})
set(sources
Expand Down
167 changes: 167 additions & 0 deletions tests/regressions/components/new_2848.cpp
@@ -0,0 +1,167 @@
// Copyright (c) 2015-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/components.hpp>
#include <hpx/include/actions.hpp>
#include <hpx/util/lightweight_test.hpp>

#include <cstddef>
#include <utility>
#include <vector>

///////////////////////////////////////////////////////////////////////////////
struct test_server : hpx::components::simple_component_base<test_server>
{
test_server() = delete;
test_server(int i) : i_(i) {}

hpx::id_type call() const { return hpx::find_here(); }

HPX_DEFINE_COMPONENT_ACTION(test_server, call);

int i_;
};

typedef hpx::components::simple_component<test_server> server_type;
HPX_REGISTER_COMPONENT(server_type, test_server);

typedef test_server::call_action call_action;
HPX_REGISTER_ACTION(call_action);

struct test_client : hpx::components::client_base<test_client, test_server>
{
typedef hpx::components::client_base<test_client, test_server> base_type;

test_client(hpx::id_type const& id)
: base_type(id)
{}
test_client(hpx::future<hpx::id_type> && id)
: base_type(std::move(id))
{}

hpx::id_type call() const
{
return hpx::async<call_action>(this->get_id()).get();
}
};

///////////////////////////////////////////////////////////////////////////////
void test_create_single_instance()
{
// make sure created objects live on locality they are supposed to be
for (hpx::id_type const& loc: hpx::find_all_localities())
{
hpx::id_type id = hpx::new_<test_server>(loc, 42).get();
HPX_TEST(hpx::async<call_action>(id).get() == loc);
}

for (hpx::id_type const& loc: hpx::find_all_localities())
{
test_client t1 = hpx::new_<test_client>(loc, 42);
HPX_TEST(t1.call() == loc);
}

// make sure distribution policy is properly used
hpx::id_type id = hpx::new_<test_server>(hpx::default_layout, 42).get();
HPX_TEST(hpx::async<call_action>(id).get() == hpx::find_here());

test_client t2 = hpx::new_<test_client>(hpx::default_layout, 42);
HPX_TEST(t2.call() == hpx::find_here());

for (hpx::id_type const& loc: hpx::find_all_localities())
{
hpx::id_type id =
hpx::new_<test_server>(hpx::default_layout(loc), 42).get();
HPX_TEST(hpx::async<call_action>(id).get() == loc);
}

for (hpx::id_type const& loc: hpx::find_all_localities())
{
test_client t3 = hpx::new_<test_client>(hpx::default_layout(loc), 42);
HPX_TEST(t3.call() == loc);
}
}

///////////////////////////////////////////////////////////////////////////////
void test_create_multiple_instances()
{
// make sure created objects live on locality they are supposed to be
for (hpx::id_type const& loc: hpx::find_all_localities())
{
std::vector<hpx::id_type> ids =
hpx::new_<test_server[]>(loc, 10, 42).get();
HPX_TEST_EQ(ids.size(), std::size_t(10));

for (hpx::id_type const& id: ids)
{
HPX_TEST(hpx::async<call_action>(id).get() == loc);
}
}

for (hpx::id_type const& loc: hpx::find_all_localities())
{
std::vector<test_client> ids =
hpx::new_<test_client[]>(loc, 10, 42).get();
HPX_TEST_EQ(ids.size(), std::size_t(10));

for (test_client const& c: ids)
{
HPX_TEST(c.call() == loc);
}
}

// make sure distribution policy is properly used
std::vector<hpx::id_type> ids =
hpx::new_<test_server[]>(hpx::default_layout, 10, 42).get();
HPX_TEST_EQ(ids.size(), std::size_t(10));
for (hpx::id_type const& id: ids)
{
HPX_TEST(hpx::async<call_action>(id).get() == hpx::find_here());
}

std::vector<test_client> clients =
hpx::new_<test_client[]>(hpx::default_layout, 10, 42).get();
HPX_TEST_EQ(clients.size(), std::size_t(10));
for (test_client const& c: clients)
{
HPX_TEST(c.call() == hpx::find_here());
}

for (hpx::id_type const& loc: hpx::find_all_localities())
{
std::vector<hpx::id_type> ids =
hpx::new_<test_server[]>(hpx::default_layout(loc), 10, 42).get();
HPX_TEST_EQ(ids.size(), std::size_t(10));

for (hpx::id_type const& id: ids)
{
HPX_TEST(hpx::async<call_action>(id).get() == loc);
}
}

for (hpx::id_type const& loc: hpx::find_all_localities())
{
std::vector<test_client> ids =
hpx::new_<test_client[]>(hpx::default_layout(loc), 10, 42).get();
HPX_TEST_EQ(ids.size(), std::size_t(10));

for (test_client const& c: ids)
{
HPX_TEST(c.call() == loc);
}
}
}

///////////////////////////////////////////////////////////////////////////////
int main()
{
test_create_single_instance();
test_create_multiple_instances();

return 0;
}