[libcxx-commits] [PATCH] D130404: [libc++][ranges] implement `std::ranges::unique{_copy}`

Hui via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jul 25 09:40:56 PDT 2022


huixie90 created this revision.
huixie90 added reviewers: philnik, var-const, ldionne.
Herald added a project: All.
huixie90 updated this revision to Diff 447097.
huixie90 added a comment.
huixie90 updated this revision to Diff 447152.
huixie90 updated this revision to Diff 447367.
huixie90 edited the summary of this revision.
huixie90 published this revision for review.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

fix CI


huixie90 added a comment.

refactor unique_copy


huixie90 added a comment.

implement unique_copy


implement `std::ranges::unique` and `std::ranges::unique_copy`

Several changes:

1. implement `std::ranges::unique` that delegates to `std::unique`

2. fix classic `std::unique_copy` does not work with output iterator that satisfies both `InputIterator` and `OutputIterator` requirements.

background:  The classic implementation does tag dispatch on the `iteartor_category` of the output iterator and uses `output_iterator_tag` as a fallback. However, if the user passes an output iterator that satisfy both 
`InputIterator` and `OutputIterator` requirement, the `iteartor_category` is likely to be `input_iteartor_tag` and `input_iteartor_tag` does not derive from `output_iterator_tag`. So the fallback case would not work.

See this example where msvc stl correctly implemented and libc++ errors out
https://godbolt.org/z/7shKshEPa

The fix to the classic algorithm is strictly following the wording

> If InputIterator meets the Cpp17ForwardIterator requirements, then there are no additional requirements for T.

Having an overload that takes `__reread_from_input_tag`, which does `auto prev = in; comp(*prev, *in)`

> Otherwise, if OutputIterator meets the Cpp17ForwardIterator requirements and its value type is the same as T, then T meets the Cpp17CopyAssignable (Table 33) requirements.

Having an overload that takes `__reread_from_output_tag`, which does `comp(*out, *in)`

> Otherwise, T meets both the Cpp17CopyConstructible (Table 31) and Cpp17CopyAssignable requirements.

Having an overload that takes `__read_from_tmp_value_tag`, which does `value_type tmp = *in;  ++in; comp(tmp, *in)`

3. implement `std::ranges::unique_copy`, which reuses the implementation above, but slightly different dispatching condition as ranges version has slightly different `requires` constraints.

Also I believe the `std::ranges::unique_copy` is incorrectly constrained. More specifically, the intent  of the constraint `(input_­iterator<O> && same_­as<iter_value_t<I>, iter_value_t<O>>)` was to allow implementations to write `comp(*in ,*out)` (ignore `std::invoke` and `projection` for now). However, `I` and `O` having the same `value_type` is not enough to allow `comp(*in ,*out)`, because that expression only uses the `iter_reference_t` of the `I` and `O`, instead of the `iter_value_t`. See discussion here
https://discord.com/channels/737189251069771789/737189251497721887/1000875764650094692

But anyway, I decided that in this patch, implement what exactly in the spec and bring up the issue in LWG later.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130404

Files:
  libcxx/docs/Status/RangesAlgorithms.csv
  libcxx/include/__algorithm/adjacent_find.h
  libcxx/include/__algorithm/iterator_operations.h
  libcxx/include/__algorithm/ranges_unique.h
  libcxx/include/__algorithm/ranges_unique_copy.h
  libcxx/include/__algorithm/unique.h
  libcxx/include/__algorithm/unique_copy.h
  libcxx/include/algorithm
  libcxx/test/libcxx/algorithms/ranges_robust_against_copying_comparators.pass.cpp
  libcxx/test/libcxx/algorithms/ranges_robust_against_copying_projections.pass.cpp
  libcxx/test/std/algorithms/alg.modifying.operations/alg.unique/ranges_unique.pass.cpp
  libcxx/test/std/algorithms/alg.modifying.operations/alg.unique/ranges_unique_copy.pass.cpp
  libcxx/test/std/algorithms/alg.modifying.operations/alg.unique/unique_copy.pass.cpp
  libcxx/test/std/algorithms/ranges_robust_against_dangling.pass.cpp
  libcxx/test/std/algorithms/ranges_robust_against_nonbool_predicates.pass.cpp
  libcxx/test/std/algorithms/ranges_robust_against_omitting_invoke.pass.cpp
  libcxx/test/std/algorithms/ranges_robust_against_proxy_iterators.pass.cpp
  libcxx/test/std/library/description/conventions/customization.point.object/niebloid.compile.pass.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130404.447367.patch
Type: text/x-patch
Size: 61081 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220725/226df928/attachment-0001.bin>


More information about the libcxx-commits mailing list