[libcxx-commits] [PATCH] D142294: [libc++] Allow transforming to a different type in transform_inclusive_scan

Hui via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sun Jan 22 10:49:25 PST 2023


huixie90 added inline comments.


================
Comment at: libcxx/include/__numeric/transform_inclusive_scan.h:48
         if (++__first != __last)
             return _VSTD::transform_inclusive_scan(__first, __last, __result, __b, __u, __init);
     }
----------------
Mordante wrote:
> I'm not sure this approach is valid.
> http://eel.is/c++draft/transform.inclusive.scan#3.1
> ```
> If init is provided, T meets the Cpp17MoveConstructible (Table 32) requirements; otherwise, U meets the Cpp17MoveConstructible requirements.
> ```
> If U (decltype(first)) is movable but T (decltype(__init)) isn't this code may violate the precondition. We even require `__init` to be copyable.
> 
> I wonder whether that's a bug in the Standard, since I'm unsure how to implement the init version with a non-copyable type. But at least here a copy feels wrong to me.
If `T` is provided, the spec mandates `binary_­op(init, unary_­op(*first))` are convertible to `T`, and `T` meets the Cpp17MoveConstructible. so 
```
__init = __b(__init, __u(*__first));
```
is valid expression (rhs expression convertible to `T`, and then a move construction)

if `T` is not provided, `binary_­op(unary_­op(*first), unary_­op(*first))` is convertible to `U`, and `U` meets the Cpp17MoveConstructible. (note `U` is `iterator_traits<Iter>::value_type` here. So I think the spec only guarantees that

```
value_type val = ...;
decltype(auto) tmp = *first; // not quite, I think if we had range version, we need `iter_common_reference` for this to be valid.
val = __b(__u(tmp), __u(*++first));
```
The legacy algorithm's constraints are not very accurate in general in my opinion.

But I think @philnik 's change is an improvement in general. but yes I think we should `std::move(__init)` here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142294



More information about the libcxx-commits mailing list