[libcxx-commits] [PATCH] D118432: [libc++][ranges] Implement `indirectly_copyable{, _storable}`.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const marked an inline comment as not done.
var-const added inline comments.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/alg.req.ind.move/indirectly_movable.compile.pass.cpp:43
+
+// Can move from a pointer into an array but cannot move from an array.
 static_assert( std::indirectly_movable<int*, int[2]>);
----------------
var-const wrote:
> Quuxplusone wrote:
> > You certainly //can// move from an array (element), so this comment is wrong.
> > My intuition says that the real hurdle here is that an array type is not an iterator type, so it's not `indirectly`-anything — it has no `iter_value_t`. (But then, an `optional` isn't an iterator either, and I think `optional`s are `indirectly`-fooable, so my intuitive explanation may be wrong.)
> I'm actually not sure about the behavior here.
> 
> Intuitively, it would seem to me that:
> - `indirectly_movable<int[2], int*>` _should_ work due to array-to-pointer decay. After all, just like you said, you can move from an array -- `int *ptr, arr[2]; *ptr = std::move(*arr);` is certainly valid;
> - in either case, I would expect this to be commutative -- i.e., `indirectly_movable<int*, int[2]>` and `indirectly_movable<int[2], int*>` should either both be true or both be false.
> 
> > My intuition says that the real hurdle here is that an array type is not an iterator type, so it's not `indirectly`-anything — it has no `iter_value_t`.
> 
> `iter_value_t` does in fact work on array types -- `static_assert(is_same_v<iter_value_t<int[2]>, int>);` is successful with both libc++ and libstdc++, at least.
> 
> Digging a little into this, it appears that for `indirectly_movable<int[2], int*>` is false because it is indirectly writable but not indirectly readable. Reducing as much as possible, it boils down to:
> ```
> template<class In>
> concept simplified_indirectly_readable =
>   requires(const In i) {
>     { *i } -> same_as<iter_reference_t<In>>;
>   };
> 
> static_assert( simplified_indirectly_readable<int*>);
> static_assert(!simplified_indirectly_readable<int[2]>);
> ```
> (there's also `{ ranges::iter_move(i) } -> same_as<iter_rvalue_reference_t<In>>;` failing for the same reason, so it can be ignored)
> 
> The reason this check:
> ```
> { *i } -> same_as<iter_reference_t<In>>
> ```
> works for pointers but fails for arrays comes down to template argument deduction. The `const In i` argument decays from array to pointer as expected; however:
> - when `In` is a pointer, the `const` in the signature "attaches" to the pointer itself, so that it is a const pointer to a non-const object (`int* const`);
> - when `In` is an array, the `const` in the signature "attaches" to the type referred to, so that it's a const array that decays to a non-const pointer to a const object (`const int*`).
> 
> For a quick illustration:
> ```
> template <class T>
> void Foo(const T x) {
>   static_assert(is_same_v<decltype(x), const int*>);
> }
> 
> template <class T>
> void Bar(const T x) {
>   static_assert(is_same_v<decltype(x), int* const>);
> }
> 
> void Test() {
>   int arr[2];
>   Foo<int[2]>(arr);
>   
>   int* ptr = nullptr;
>   Bar<int*>(ptr);
> }
> ```
> ([link](https://wandbox.org/permlink/vIKFC4fnW1sPbfzS))
> 
> It seems to me that this is pretty subtle and I wonder whether this is intentional -- basically, whether arrays are intentionally not `indirectly_readable`, or it's just a by-product of the implementation. If arrays are intentionally `not_readable`, it seems like they should be excluded in a more obvious way.
> 
> Let me know what you think (and if you see any errors in my reasoning).
@Quuxplusone Any thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118432