[libcxx-commits] [PATCH] D102639: [libcxx][ranges] Add `indirectly_­movable` and `indirectly_movable_storable`.

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver marked 8 inline comments as done.
zoecarver added a comment.

@ldionne applying your comments now...

---

> I have no idea what MoveOnlyConvertibleWrapper is supposed to represent. Can you please give all your wrapper types more descriptive names?

What would you suggest I name something like `MoveOnlyConvertibleWrapper`? Does `ConvertibleToMoveOnlyWrapper` work? Or is it the `Wrapper` part specifically that you don't like?

This is testing the case where we can convert to a type that is assignable to `MoveOnly` but it itself is not assignable to `MoveOnly`.



================
Comment at: libcxx/test/std/containers/associative/map/iterator_concept_conformance.compile.pass.cpp:37
+static_assert( std::indirectly_movable<iterator, std::pair<int, int>*>);
+static_assert(!std::indirectly_movable_storable<iterator, std::pair<int, int>*>);
 
----------------
ldionne wrote:
> ldionne wrote:
> > Is this because `iterator::value_type` is `std::pair<int const, int>` and not `std::pair<int, int>`?
> Per offline discussion, I think this should be using `value_type*` instead of `pair<int, int>*`.
The reason this doesn't use value_type is because it's not assignable-to. In this case value_type is `pair<const int, int>` which we can't assign `pair<const int, int>` to. But we can assign `pair<const int, int>` to `pair<int, int>` so, we use that instead (because that's the only use-case where this would succeed). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102639