Page MenuHomePhabricator
Differential D102639 Diff 345997 libcxx/test/std/iterators/iterator.requirements/alg.req/ind.move.compile.pass.cpp

Changeset View

Standalone View

View Options

libcxx/test/std/iterators/iterator.requirements/alg.req/ind.move.compile.pass.cpp

  • This file was added.
//===----------------------------------------------------------------------===//
//
ldionneUnsubmitted
Done
ReplyInline Actions

I would like us to test one concept per file, like we do everywhere else.

ldionne: I would like us to test one concept per file, like we do everywhere else.
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// UNSUPPORTED: c++03, c++11, c++14, c++17
// UNSUPPORTED: libcpp-no-concepts
// UNSUPPORTED: gcc-10
// template<class In, class Out>
// concept indirectly_movable;
MordanteUnsubmitted
Done
ReplyInline Actions

template<class In, class Out>

Mordante: `template<class In, class Out>`
// template<class In, class Out>
// concept indirectly_movable_storable;
#include <iterator>
#include "test_macros.h"
QuuxplusoneUnsubmitted
Done
ReplyInline Actions

I think we need some tests using a type with

const int& operator*() const;

or at least some tests using const int*. For example, std::indirectly_movable<const int*, int*> should be false (but std::indirectly_movable<int*, const int*> should be true). And std::indirectly_movable<MoveOnly*, const MoveOnly*> should also be false because MoveOnly is move-only.

Quuxplusone: I think we need some tests using a type with ``` const int& operator*() const; ``` or at least…
struct IndirectlyMovableWithInt {
int& operator*() const;
};
struct Empty {};
struct MoveOnlyConvertible;
struct AssignableToMoveOnly;
struct MoveOnly {
MoveOnly(MoveOnly&&) = default;
MoveOnly(MoveOnly const&) = delete;
MoveOnly& operator=(MoveOnly&&) = default;
cjdbUnsubmitted
Done
ReplyInline Actions

IndirectlyReadableMoveOnly is a more descriptive name. It's not really wrapping anything.

cjdb: `IndirectlyReadableMoveOnly` is a more descriptive name. It's not really wrapping anything.
MoveOnly& operator=(MoveOnly const&) = delete;
MoveOnly() = default;
MoveOnly& operator=(MoveOnlyConvertible const&) = delete;
MoveOnly& operator=(AssignableToMoveOnly const&);
};
cjdbUnsubmitted
Done
ReplyInline Actions

Please add tests for lvalue arrays too.

cjdb: Please add tests for lvalue arrays too.
struct MoveOnlyWrapper {
using value_type = MoveOnly;
MoveOnly& operator*() const;
};
static_assert( std::indirectly_movable<int*, int*>);
static_assert( std::indirectly_movable<const int*, int *>);
static_assert(!std::indirectly_movable<int*, const int *>);
static_assert(!std::indirectly_movable<const int*, const int *>);
static_assert( std::indirectly_movable<int*, int[2]>);
static_assert(!std::indirectly_movable<int[2], int*>);
static_assert(!std::indirectly_movable<int[2], int[2]>);
static_assert(!std::indirectly_movable<int(&)[2], int(&)[2]>);
static_assert(!std::indirectly_movable<int, int*>);
static_assert(!std::indirectly_movable<int, int>);
static_assert( std::indirectly_movable<Empty*, Empty*>);
static_assert(!std::indirectly_movable<Empty*, IndirectlyMovableWithInt>);
static_assert( std::indirectly_movable<int*, IndirectlyMovableWithInt>);
static_assert( std::indirectly_movable<MoveOnly*, MoveOnly*>);
static_assert(!std::indirectly_movable<MoveOnly*, const MoveOnly*>);
static_assert(!std::indirectly_movable<const MoveOnly*, const MoveOnly*>);
static_assert(!std::indirectly_movable<const MoveOnly*, MoveOnly*>);
static_assert( std::indirectly_movable<MoveOnlyWrapper, MoveOnlyWrapper>);
static_assert( std::indirectly_movable<MoveOnly*, MoveOnlyWrapper>);
struct MoveOnlyConvertible {
operator MoveOnly&() const;
};
struct MoveOnlyConvertibleWrapper {
using value_type = MoveOnlyConvertible;
friend MoveOnly iter_move(MoveOnlyConvertibleWrapper const&);
MoveOnly& operator*() const;
};
QuuxplusoneUnsubmitted
Not Done
ReplyInline Actions

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.

Quuxplusone: I think I agree with Chris's confusion here. Is this perhaps backwards? Did you mean to write…
struct AssignableToMoveOnly {
AssignableToMoveOnly() = default;
AssignableToMoveOnly(const MoveOnly&);
};
struct AssignableToMoveOnlyWrapper {
using value_type = AssignableToMoveOnly;
friend MoveOnly iter_move(AssignableToMoveOnlyWrapper const&);
MoveOnly& operator*() const;
};
struct DeletedMoveCtor {
DeletedMoveCtor(DeletedMoveCtor&&) = delete;
DeletedMoveCtor& operator=(DeletedMoveCtor&&) = default;
};
static_assert( std::indirectly_movable_storable<int*, int*>);
static_assert( std::indirectly_movable_storable<const int*, int *>);
static_assert( std::indirectly_movable_storable<int*, int[2]>);
static_assert( std::indirectly_movable_storable<Empty*, Empty*>);
static_assert( std::indirectly_movable_storable<MoveOnly*, MoveOnly*>);
static_assert( std::indirectly_movable<int*, IndirectlyMovableWithInt>);
static_assert( std::indirectly_movable_storable<MoveOnlyWrapper, MoveOnlyWrapper>);
static_assert( std::indirectly_movable<MoveOnlyConvertibleWrapper, MoveOnlyConvertibleWrapper>);
static_assert(!std::indirectly_movable_storable<MoveOnlyConvertibleWrapper, MoveOnlyConvertibleWrapper>);
static_assert(!std::indirectly_movable_storable<DeletedMoveCtor*, DeletedMoveCtor*>);
static_assert( std::indirectly_movable_storable<AssignableToMoveOnlyWrapper, AssignableToMoveOnlyWrapper>);
cjdbUnsubmitted
Done
ReplyInline Actions

I think you're missing a test for when exactly one of each concept is false. Also, can we get these moved into an indirectly_movable_storable.cpp test please?

cjdb: I think you're missing a test for when exactly one of each concept is false. Also, can we get…
zoecarverAuthorUnsubmitted
Done
ReplyInline Actions

Can you point out the cases that aren't covered? I think I've got them all, except maybe !constructible_from.

I think this is still a fairly small test file, and it's nice to be able to reuse types.

zoecarver: Can you point out the cases that aren't covered? I think I've got them all, except maybe `!
cjdbUnsubmitted
Done
ReplyInline Actions

I don't see cases where only:

  • indirectly_movable is false
  • indirectly_writable is false
  • constructible_from is false
  • assignable_from is false
cjdb: I don't see cases where only: * `indirectly_movable` is false * `indirectly_writable` is false…