Repository navigation
Implement ranges::replace - #983
Stephan T. Lavavej (StephanTLavavej) merged 8 commits into
Conversation
e8f280f to
1840a5d
Compare
Casey Carter (CaseyCarter)
left a comment
Casey Carter (CaseyCarter)
left a comment
There was a problem hiding this comment.
This does not yet compile as the algorithm has some additional constraints that are not fulfilled by the generic
with_input_range
Since this is passing now, I suspect you've realized that testing ranges with proxy references requires you to stick fairly closely to ints or pairs, or be prepared to add operations to test::proxy_reference.
also the wording is really strange. It says that the algorithm shall return
lastwhich could actually be a sentinel. I believe it is indeed meant to return respective iterator but icky wording.
See [algorithms.requirements]/13, and read [algorithms.requirements]/12 while you're there. WG21 specifies algorithms in a DSL that resembles but is not C++ ;)
|
One style question for the maintainers. Do you prefer individual algorithm PRs like this here or should I open a single PR for e.g. all |
|
I think it's generally easier to review an entire family of algorithms in a single PR, since following a consistent pattern makes reviewing similar algorithms easier - then we just need to watch out for inconsistencies and (somewhat harder) watch out for places that should be different - neither of which is helped by splitting algorithms across PRs. Other maintainers may feel differently. It's not critically important for me. |
d7410cc to
fe6d75c
Compare
|
To ease review and reduce thenumber of concurrent PRs I have merged the other two remaining |
* Extract `with_output_iterators` from `with_writable_iterators` and implement `test_in_outerator` to instantiate with `input_range` and `output_iterator` types. * Pull `ranges::equal` out of `instantiator::call` in both `replace_copy` and `replace_copy_if` tests to avoid `/analyze` exhausting the compiler heap.
Stephan T. Lavavej (StephanTLavavej)
left a comment
Stephan T. Lavavej (StephanTLavavej)
left a comment
There was a problem hiding this comment.
Will push changes to fix comment typos, otherwise looks great!
|
Thanks Michael Schellenberger Costa (@miscco)! You're irreplaceable. 😎 |
3 participants
This implements the
ranges::replacealgorithm.Casey Carter (@CaseyCarter): This does not yet compile as the algorithm has some additional constraints that are not fulfilled by the generic
with_input_rangeThe question is whether we want to specialize some
with_comparable_rangein the algorithm support header or simply special case it in the test. As that will most likely happen with other algorithms too I would like find a fitting solution that fits best for all algorithms.Also the wording is really strange. IT says that the algorithm shall return
lastwhich could actually be a sentinel. I believe it is indeed meant to return respective iterator but icky wording