Skip to content

Replace enable_if with HPX_CONCEPT_REQUIRES_ and add is_sentinel_for constraint#4792

Merged
hkaiser merged 2 commits into
TheHPXProject:masterfrom
gonidelis:reduce_adapt
Jun 29, 2020
Merged

Replace enable_if with HPX_CONCEPT_REQUIRES_ and add is_sentinel_for constraint#4792
hkaiser merged 2 commits into
TheHPXProject:masterfrom
gonidelis:reduce_adapt

Conversation

@gonidelis
Copy link
Copy Markdown
Contributor

@gonidelis gonidelis commented Jun 26, 2020

I replaced enable_if with the HPX_CONCEPT_REQUIRES_ macro and added is_sentinel_for trait (except for the segmented algorithm part). All pre-existing tests seem to pass.

@hkaiser hkaiser added this to the 1.5.0 milestone Jun 26, 2020
hkaiser
hkaiser previously approved these changes Jun 26, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Jun 27, 2020

@gonidelis I added one commit to this PR that improves the readability of the HPX_CONCEPT_REQUIRES_() expressions and disables formatting for them.

@gonidelis
Copy link
Copy Markdown
Contributor Author

gonidelis commented Jun 28, 2020

@gonidelis I added one commit to this PR that improves the readability of the HPX_CONCEPT_REQUIRES_() expressions and disables formatting for them.

Ok that's great! I didn't know about the clang-format on/off option. Looks better now for sure!

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Jun 28, 2020

I have still missed one place. Would you mind adding it here as well: https://github.com/STEllAR-GROUP/hpx/pull/4792/files#diff-ba8f23897c313b29d2dc80aa94393b40R87?

@gonidelis
Copy link
Copy Markdown
Contributor Author

gonidelis commented Jun 28, 2020

I have still missed one place. Would you mind adding it here as well: https://github.com/STEllAR-GROUP/hpx/pull/4792/files#diff-ba8f23897c313b29d2dc80aa94393b40R87?

You mean I should add // clang-format on/of, right?

@hkaiser hkaiser merged commit 5d5d787 into TheHPXProject:master Jun 29, 2020
aurianer
aurianer reviewed Jul 1, 2020
View reviewed changes
Comment thread libs/algorithms/include/hpx/parallel/algorithms/reduce.hpp
@gonidelis gonidelis deleted the reduce_adapt branch July 22, 2020 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hkaiser hkaiser hkaiser left review comments

@biddisco biddisco Awaiting requested review from biddisco

@msimberg msimberg Awaiting requested review from msimberg

+1 more reviewer

@aurianer aurianer aurianer left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

category: algorithms type: compatibility issue type: enhancement

Projects

None yet

Milestone

1.5.0

Development

Successfully merging this pull request may close these issues.

3 participants

@gonidelis @hkaiser @aurianer