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

Adapt execution policy name changes from C++17 #2436

Merged
merged 8 commits into from Dec 31, 2016
Merged

Adapt execution policy name changes from C++17 #2436

merged 8 commits into from Dec 31, 2016

Conversation

hkaiser
Copy link
Member

@hkaiser hkaiser commented Dec 22, 2016

This fixes #2390

- those are now in namespace hpx::parallel::execution (per C++17)
- added compatibility option to define aliases representing old names
@@ -24,18 +24,18 @@
#include <type_traits>
#include <utility>

namespace hpx { namespace parallel { HPX_INLINE_NAMESPACE(v1)
namespace hpx { namespace parallel { namespace execution
Copy link
Member

Choose a reason for hiding this comment

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

why is this one not versioned anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is versioned. In full correspondence with the proposal: hpx::parallel::execution::concurrency_v2 (there it is std::execution::concurrency_v2).

Copy link
Member

Choose a reason for hiding this comment

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

Ok. I clearly miss where the versioning is done here. I miss the inline namespace v2 or something similar.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, my misunderstanding - I mixed that up with the executor work I'm doing right now. Please disregard my first comment.

I assume, you are talking about moving (keeping) the datapar execution policies into a versioned namespace. I agree, that should probably be done.

# HPX_WITH_EXECUTION_POLICY_COMPATIBILITY: introduced in V1.0.0
hpx_option(HPX_WITH_EXECUTION_POLICY_COMPATIBILITY BOOL
"Enable old execution policy names in API (default: OFF)"
OFF ADVANCED)
Copy link
Member

Choose a reason for hiding this comment

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

Can we have this on by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. Good point.

@@ -78,6 +79,7 @@ The options are split into these categories:
[[[#build_system.cmake_variables.HPX_WITH_DATAPAR_BOOST_SIMD] `HPX_WITH_DATAPAR_BOOST_SIMD:BOOL`][Enable data parallel algorithm support using the external Boost.SIMD library (default: OFF)]]
[[[#build_system.cmake_variables.HPX_WITH_DATAPAR_LIBFLATARRAY] `HPX_WITH_DATAPAR_LIBFLATARRAY:BOOL`][Enable data parallel algorithm support using the external LibFlatArray (default: OFF)]]
[[[#build_system.cmake_variables.HPX_WITH_DATAPAR_VC] `HPX_WITH_DATAPAR_VC:BOOL`][Enable data parallel algorithm support using the external Vc library (default: OFF)]]
[[[#build_system.cmake_variables.HPX_WITH_EXECUTION_POLICY_COMPATIBILITY] `HPX_WITH_EXECUTION_POLICY_COMPATIBILITY:BOOL`][Enable old execution policy names in API (default: OFF)]]
Copy link
Member

Choose a reason for hiding this comment

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

the new default should be reflected here

Copy link
Member Author

@hkaiser hkaiser Dec 29, 2016

Choose a reason for hiding this comment

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

Right.

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW, I think we should remove this file from the repository altogether as it is regenerated by cmake.

@hkaiser
Copy link
Member Author

hkaiser commented Dec 30, 2016

Can I go ahead and merge this now?

@hkaiser
Copy link
Member Author

hkaiser commented Dec 31, 2016

The comments left by @sithhell have been addressed.

@hkaiser hkaiser merged commit a320121 into master Dec 31, 2016
@hkaiser hkaiser deleted the fixing_2390 branch December 31, 2016 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Standard Algorithms
  
Merged to master
Development

Successfully merging this pull request may close these issues.

Adapt execution policy name changes from C++17
2 participants