Adapted parallel::{search | search_n} for Ranges TS (see #1668) #3210
Conversation
…Updated compare projection to take two different types of projections
hpx/parallel/algorithms/search.hpp
Outdated
| traits::is_projected<Proj1, FwdIter>::value && | ||
| hpx::traits::is_iterator<FwdIter2>::value && | ||
| traits::is_projected<Proj2, FwdIter2>::value && | ||
| std::is_convertible< |
There was a problem hiding this comment.
Hmm.. I don't think that FwdIter2's thing and FwdIter's thing must be convertible each other because BinaryPredicate doesn't require 'strict weak ordering' unlike Compare concepts.
But I'm not sure. I can't find the clarified thing in c++ standard draft document...
But, the standard just says
In other words, if an algorithm takes BinaryPredicate binary_pred as its argument and first1 and first2 as its iterator arguments, it should work correctly in the construct binary_pred(*first1, *first2) contextually converted to bool (Clause 4).
Page 896 of http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4296.pdf
There was a problem hiding this comment.
So Clause 4 discusses implicit conversions which is why I think that addition is necessary.
There was a problem hiding this comment.
@cogle Clause 4 discusses about conversion to Boolean value. This is not related to our focus. (The sentence what I refer is not 'Clause 4'.)
There was a problem hiding this comment.
@cogle The sentence what I refer just says about binary_pred(*first1, *first2). There is no content about binary_pred(*first2, *first1).
There was a problem hiding this comment.
I see. I asked stack overflow about this to see if somebody on there had any insight I to why that site would explicitly state that the two must be implicitly convertible, and will update with and answer if I get one.
hpx/parallel/algorithms/search.hpp
Outdated
| @@ -98,7 +118,8 @@ namespace hpx {namespace parallel { inline namespace v1 | |||
| local_count != diff && len != count; | |||
| ++local_count, ++len, ++mid) | |||
| { | |||
| if(*mid != *++needle) | |||
| if(hpx::util::invoke(proj1, *mid) != | |||
There was a problem hiding this comment.
Using op rather than != seems correct.
hpx/parallel/algorithms/search.hpp
Outdated
| traits::is_projected<Proj1, FwdIter>::value && | ||
| hpx::traits::is_iterator<FwdIter2>::value && | ||
| traits::is_projected<Proj2, FwdIter2>::value && | ||
| std::is_convertible< |
|
@taeguk let me know of any other issues such as poor formatting or other misc things. |
This is further continuation of my work with regards to issue #1668. For this particular pull request please take your time with the review process as this is my largest contribution to the library thus far, and I did make some changes to the existing library most notably, to
hpx/parallel/util/compare_projected.hppin order to extend it such that it can take two distinct projections. In addition I added an extra constraint to the search(_n) algorithm's template parameter list, enforcing that the two respective iterators are implicitly convertible which is a constraint stated here.Edit: I have since removed the implicit convertible check as it appears after consulting with stack overflow that this an error in the website's template.