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

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/alg.req/ind.move.compile.pass.cpp:72-76
+struct MoveOnlyConvertibleWrapper {
+  using value_type = MoveOnlyConvertible;
+  friend MoveOnly iter_move(MoveOnlyConvertibleWrapper const&);
+  MoveOnly& operator*() const;
+};
----------------
I think I agree with Chris's confusion here. Is this perhaps backwards? Did you mean to write
```
    using value_type = MoveOnly;
    MoveOnlyConvertible operator*() const;
```
so that this would be a lot like `vector<bool>` (i.e. its `operator*` returns a proxy convertible to `value_type`)? If so, I think `VectorBool` might be a decent name for it.

The problem with stringing particles together, like `MoveOnly+Convertible+Wrapper`, is that it's never clear what particle refers to what. Is this a wrapper around a MoveOnlyConvertible? Is it a wrapper that's convertible to a MoveOnly? Is it a move-only wrapper that is also convertible? Is it move-only wrapper around a Convertible? etc.  It would be strictly less confusing to just name it `struct A`.

Another trick that can help with readability is to use nested types to keep related entities tightly coupled. If the only defining characteristic of `MoveOnlyConvertible` is that you can convert from it to `MoveOnly`, then it should probably be named `MoveOnly::From`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102639