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.

Sign up for GitHub

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

Jump to bottom

Fixing shared_executor_test #1992

Merged
merged 3 commits into from Feb 20, 2016
Merged

Fixing shared_executor_test #1992

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fa5a3c6
Fixing shared_executor_test
hkaiser Feb 18, 2016
af36acf
Moving solution to shared_executor problem to traits
hkaiser Feb 18, 2016
45f26dd
Properly handle void return types, adding tests
hkaiser Feb 19, 2016
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
1 change: 1 addition & 0 deletions hpx/traits/acquire_shared_state.hpp
View file
Open in desktop
Expand Up @@ -68,6 +68,7 @@ namespace hpx { namespace traits
struct acquire_shared_state_impl<T,
typename boost::disable_if_c<is_future_or_future_range<T>::value>::type>
{
typedef T type;

template <typename T_>
HPX_FORCEINLINE
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/parallel/executors/shared_parallel_executor.cpp
View file
Open in desktop
Expand Up @@ -11,6 +11,7 @@
#include <algorithm>
#include <cstdlib>
#include <vector>
#include <type_traits>

#include <boost/range/functions.hpp>

Expand All @@ -32,6 +33,17 @@ struct shared_parallel_executor
{
return hpx::async(std::forward<F>(f));
}

template <typename F>
typename std::remove_reference<
typename hpx::util::result_of<
typename hpx::util::decay<F>::type()
>::type
>::type
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be rather done in the trait? Is it an error in general to return a reference from the passed function?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried solving it in the trait, but was not able to do it there. If you have an idea how this could be formulated, I'd appreciate any ideas.

Copy link
Member

Choose a reason for hiding this comment

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

I tried solving it in the trait, but was not able to do it there. If you
have an idea how this could be formulated, I'd appreciate any ideas.

I am still trying to wrap my around why this is needed in the first place.

Copy link
Member

Choose a reason for hiding this comment

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

Tampering with the result like this looks wrong to me. It will also make some valid corner cases ill-formed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. Any suggestions on how to solve this? The patch here was just meant as a stop-gap measure to make tests pass. Any proper solution would be certainly preferred.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried solving it in the trait, but was not able to do it there. If you
have an idea how this could be formulated, I'd appreciate any ideas.

I am still trying to wrap my around why this is needed in the first place.

shared_future<>::get() returns a const& to the result. This code extracts the result, passing it along, the shared_future itself however goes out of scope.

execute(F && f)
{
return async_execute(std::forward<F>(f)).get();
}
};

///////////////////////////////////////////////////////////////////////////////
Expand Down