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

Add projection function to find_* (and fix very bad bug) #3436

Merged
merged 6 commits into from
Sep 10, 2018

Conversation

brjsp
Copy link
Contributor

@brjsp brjsp commented Aug 31, 2018

This started as an attempt to make find_* Ranges TS compliant (see #1668). But during testing, i discovered and fixed a bad bug in find_end making it unusable with custom comparison operator.
Because of this bug, i am making this pull request now, before writing a container version of the above algorithms. The regular versions are already usable as given in this pull request.

@hkaiser
Copy link
Member

hkaiser commented Aug 31, 2018

Thanks a lot!

Please also fix the inspect errors reported here: https://30382-4455628-gh.circle-artifacts.com/0/hpx_inspect_report.html

@brjsp
Copy link
Contributor Author

brjsp commented Aug 31, 2018

After a discussion with @K-ballo on chat, i have removed plain find from the pull request, because it is really hard to do correctly (and in a way not requiring C++14).

@brjsp brjsp force-pushed the rangests branch 2 times, most recently from 87fb532 to cdb5f7a Compare September 1, 2018 10:54
@brjsp
Copy link
Contributor Author

brjsp commented Sep 1, 2018

Actually doing a projection version of find_if and find_if_not would mean having to do also do this for the segmented algorithms, so not doing it now.

@hkaiser
Copy link
Member

hkaiser commented Sep 5, 2018

@brjsp thanks for working on this. I have not followed all the discussion around this - could you give a short overview about the current state of this PR, please?

@brjsp
Copy link
Contributor Author

brjsp commented Sep 5, 2018

@hkaiser find_end and find_first_of have added projection and range version, the bug in find_end is fixed, other functions are untouched, should be ready for merging.

hkaiser
hkaiser previously approved these changes Sep 9, 2018
Copy link
Member

@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 a lot!

@hkaiser
Copy link
Member

hkaiser commented Sep 9, 2018

@msimberg I think we can go ahead and merge this. There shouldn't be any interaction with other parts of HPX besides the algorithms themselves.

@msimberg
Copy link
Contributor

msimberg commented Sep 9, 2018

@hkaiser yeah, this looks safe to merge. @brjsp could I just ask you to add hpx/parallel/container_algorithms/find.hpp here. Thanks a lot for your work!

@hkaiser hkaiser merged commit 71c7c26 into STEllAR-GROUP:master Sep 10, 2018
@brjsp brjsp deleted the rangests branch September 10, 2018 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants