[libcxx-commits] [PATCH] D98870: [DO NOT LAND] [libc++] [P2248] "Enabling list-initialization for algorithms"

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/algorithm:2053
+#if _LIBCPP_STD_VER > 20
+    = __iter_value_type<_OutputIterator>  // [ajo] P2248R1 says OutputIterator, but should be InputIterator
+#endif
----------------
curdeius wrote:
> Out of curiosity, what's your reasoning on this? Why input output should be preferred?
> Do you mean some fancy output iterators that accept any type but e.g. do some counting and, hence, have a different value_type?
Let `I` be `__iter_value_type<_InputIterator>` and `O` be `__iter_value_type<_OutputIterator>`.

The only thing we do with `__old_value` is compare it to an object of type `I`. There's no particular reason to think that `I` is comparable with `O`, nor to think that `I` is comparable with `I`... but it seems //more// reasonable to compare `I` with `I` than to compare `I` with `O`.

The only thing we do with `__new_value` is assign it into `*__result` — something we already do with a value of type `I` (line 2065) but never with a value of type `O`.

Also consider this contrived example: `std::replace_copy(floatpairs.begin(), floatpairs.end(), intpairs.begin(), {1.5, 1.5}, {2,2})`.


================
Comment at: libcxx/include/algorithm:4435
 
-template <class _ForwardIterator, class _Tp, class _Compare>
+template <class _ForwardIterator, class _Compare, class _Tp
+#if _LIBCPP_STD_VER > 20
----------------
curdeius wrote:
> Is switching the order necessary? It's not in the paper.
> And it's a function template so there is no restriction on parameters following the defaulted one.
It's not necessary, but we're free to do it, and I decided I didn't like the look of
```
#endif
, class _Compare>
```
with that dangling comma.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98870/new/

https://reviews.llvm.org/D98870