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

Konstantin Varlamov via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Jan 27 22:03:27 PST 2022


var-const added inline comments.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/alg.req.ind.move/indirectly_movable.compile.pass.cpp:19-20
 
-struct IndirectlyMovableWithInt {
-  int& operator*() const;
-};
----------------
It seems to be exactly the same as `PointerTo<int>`.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/alg.req.ind.move/indirectly_movable.compile.pass.cpp:52
-static_assert(!std::indirectly_movable<Empty*, IndirectlyMovableWithInt>);
-static_assert( std::indirectly_movable<int*, IndirectlyMovableWithInt>);
 static_assert( std::indirectly_movable<MoveOnly*, MoveOnly*>);
----------------
This line was redundant.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/alg.req.ind.move/indirectly_movable_storable.compile.pass.cpp:41
 
-// MoveOnlyConvertible is convertible to MoveOnly, but not assignable to it. This is
-// implemented by explicitly deleting "operator=(MoveOnlyConvertible)" in MoveOnly.
----------------
IIUC, the details about conversions are the only interesting things about this class -- it doesn't have to be move-only and doesn't relate to `MoveOnly`. It seems simpler to create a separate group of classes rather than attach it to the existing `MoveOnly`.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/alg.req.ind.move/indirectly_movable_storable.compile.pass.cpp:74-119
 template <template <class> class X, template <class> class Y>
 struct std::basic_common_reference<NotConstructibleFromRefIn::ValueType,
                                    NotConstructibleFromRefIn::ReferenceType, X, Y> {
   using type = CommonType&;
 };
 
 template <template <class> class X, template <class> class Y>
----------------
While I think I understand the original intention here, specializing `basic_common_reference` doesn't seem to have any effect on the test (the test passes as is without the specialization and would fail if there is a conversion between `ValueType` and `ReferenceType` regardless of the specialization).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118432



More information about the libcxx-commits mailing list