[libcxx] adds concept std::movable

[cjdb]

Summary

Implements parts of:

• P0898R3 Standard Library Concepts
• P1754 Rename concepts to standard_case for C++20, while we still can

Depends on D97162

[Mordante]

I had a quick look, I'll have a closer look later.

libcxx/include/concepts

330

T -> _Tp

[cjdb]

uglifies names

[Mordante]

Some small issues, but like to see the build pass before approving.

libcxx/test/std/concepts/object/movable.compile.pass.cpp

13

Please remove class From, .

22

I see the single thread build node is failing. Can you guard the usage of types defined in this header to be excluded if libc++ is build without thread support.

[cjdb]

attempts @Mordante's feedback

[cjdb]

properly applies @Mordante's feedback

[cjdb]

adds more types to test

[cjdb]

adds new types to test

[cjdb]

rebases to activate CI

[cjdb]

rebases and fixes things up to keep in line with prior commits

[cjdb]

renames object.h to movable.h

[cjdb]

gets this passing CI

[• Quuxplusone]

As usual, the code in <concepts> is "obviously correct" (and the tests are more verbose than I'd wish, but I don't intend to hold it up over that). LGTM%nits, if tests pass.

libcxx/test/std/concepts/object/movable.compile.pass.cpp

114

I don't see what these cases have to do with the concept; aren't these just a list of ways a type can fail to be assignable at all? I'd just write one non-assignable type right here in this file and then test that one.
But I guess since you've already written all the code, maybe it's too late to ask for it to be shortened (or maybe you only lengthened it in the first place due to someone else's feedback, I dunno).

libcxx/test/std/concepts/object/movable.h

49 ↗

(On Diff #328728)

This is not a move constructor.
Also, I don't know what the overload set ends up looking like here; do you expect there to be a defaulted operator=(const_move_constructor&&) as well? What about a defaulted operator=(const const_move_constructor&)?

[cjdb]

applies some of @Quuxplusone's feedback

libcxx/test/std/concepts/object/movable.compile.pass.cpp

13

Fixed properly this time.

114

I think we're in agreement that these can stay per https://reviews.llvm.org/D97443#inline-920565.

libcxx/test/std/concepts/object/movable.h

49 ↗

(On Diff #328728)

This is not a move constructor.

Wow.

Also, I don't know what the overload set ends up looking like here; do you expect there to be a defaulted operator=(const_move_constructor&&) as well? What about a defaulted operator=(const const_move_constructor&)?

I only expected this overload, since it's a user-declared move assignment operator. I've added a few new tests around this topic.

[cjdb]

rebases to activate CI

[zoecarver]

libcxx/test/std/concepts/object/movable.h

71 ↗

(On Diff #328830)

I think it would be good to run added tests through clang-format. (If this is what clang-format generated, no worries.)

[cjdb]

libcxx/test/std/concepts/object/movable.h

71 ↗

(On Diff #328830)

arc diff won't let you submit without first committing the changes clang-format makes :-)
(FWIW I think our .clang-format is very underspecified.)

[zoecarver]

libcxx/test/std/concepts/object/movable.h

71 ↗

(On Diff #328830)

Fair enough. Thanks :)

[curdeius]

libcxx/test/std/concepts/object/movable.h

71 ↗

(On Diff #328830)

You always can use arc diff --nolint.

[cjdb]

applies @zoecarver's feedback on headers from D97443

[EricWF]

LGTM. Nice tests, and thank you for moving the test header into support.

libcxx/include/concepts

 // [concept.copyconstructible]
 template<class _Tp>
 concept copy_constructible =
   move_constructible<_Tp> &&
   constructible_from<_Tp, _Tp&> && convertible_to<_Tp&, _Tp> &&
   constructible_from<_Tp, const _Tp&> && convertible_to<const _Tp&, _Tp> &&
   constructible_from<_Tp, const _Tp> && convertible_to<const _Tp, _Tp>;
 
+// [concepts.object]
+template<class _Tp>
+concept movable =
+  is_object_v<_Tp> &&
+  move_constructible<_Tp> &&
+  assignable_from<_Tp&, _Tp> &&
+  swappable<_Tp>;
+
 // [concept.invocable]
 template<class _Fn, class... _Args>
 concept invocable = requires(_Fn&& __fn, _Args&&... __args) {
   _VSTD::invoke(_VSTD::forward<_Fn>(__fn), _VSTD::forward<_Args>(__args)...); // not required to be equality preserving
 };
 
 // [concept.regular.invocable]
 template<class _Fn, class... _Args>

libcxx/test/std/concepts/object/movable.compile.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// template<class From>
+// concept movable = see below;
+
+#include <concepts>
+
+#include <deque>
+#include <forward_list>
+#include <list>
+#include <map>
+#include <mutex>
+#include <optional>
+#include <unordered_map>
+#include <vector>
+
+// Movable types
+static_assert(std::movable<int>);
+static_assert(std::movable<int volatile>);
+static_assert(std::movable<int*>);
+static_assert(std::movable<int const*>);
+static_assert(std::movable<int volatile*>);
+static_assert(std::movable<int const volatile*>);
+static_assert(std::movable<int (*)()>);
+
+struct S {};
+static_assert(std::movable<S>);
+static_assert(std::movable<int S::*>);
+static_assert(std::movable<int (S::*)()>);
+static_assert(std::movable<int (S::*)() noexcept>);
+static_assert(std::movable<int (S::*)() &>);
+static_assert(std::movable<int (S::*)() & noexcept>);
+static_assert(std::movable<int (S::*)() &&>);
+static_assert(std::movable<int (S::*)() && noexcept>);
+static_assert(std::movable<int (S::*)() const>);
+static_assert(std::movable<int (S::*)() const noexcept>);
+static_assert(std::movable<int (S::*)() const&>);
+static_assert(std::movable<int (S::*)() const & noexcept>);
+static_assert(std::movable<int (S::*)() const&&>);
+static_assert(std::movable<int (S::*)() const && noexcept>);
+static_assert(std::movable<int (S::*)() volatile>);
+static_assert(std::movable<int (S::*)() volatile noexcept>);
+static_assert(std::movable<int (S::*)() volatile&>);
+static_assert(std::movable<int (S::*)() volatile & noexcept>);
+static_assert(std::movable<int (S::*)() volatile&&>);
+static_assert(std::movable<int (S::*)() volatile && noexcept>);
+static_assert(std::movable<int (S::*)() const volatile>);
+static_assert(std::movable<int (S::*)() const volatile noexcept>);
+static_assert(std::movable<int (S::*)() const volatile&>);
+static_assert(std::movable<int (S::*)() const volatile & noexcept>);
+static_assert(std::movable<int (S::*)() const volatile&&>);
+static_assert(std::movable<int (S::*)() const volatile && noexcept>);
+
+#ifndef _LIBCPP_HAS_NO_THREADS
+using std::mutex;
+#else
+struct mutex {
+  mutex(mutex&&) = delete;
+  mutex& operator=(mutex&&) = delete;
+};
+#endif
+
+static_assert(std::movable<std::deque<int> >);
+static_assert(std::movable<std::forward_list<int> >);
+static_assert(std::movable<std::list<int> >);
+static_assert(std::movable<std::map<mutex, mutex> >);
+static_assert(std::movable<std::optional<std::vector<int> > >);
+static_assert(std::movable<std::unordered_map<mutex, mutex> >);
+static_assert(std::movable<std::vector<int> >);
+static_assert(std::movable<std::vector<mutex> >);
+
+struct traditional_copy_assignment_only {
+  traditional_copy_assignment_only&
+  operator=(traditional_copy_assignment_only const&);
+};
+static_assert(std::is_move_assignable_v<traditional_copy_assignment_only>);
+static_assert(std::movable<traditional_copy_assignment_only>);
+
+// Not objects
+static_assert(!std::movable<int&>);
+static_assert(!std::movable<int const&>);
+static_assert(!std::movable<int volatile&>);
+static_assert(!std::movable<int const volatile&>);
+static_assert(!std::movable<int&&>);
+static_assert(!std::movable<int const&&>);
+static_assert(!std::movable<int volatile&&>);
+static_assert(!std::movable<int const volatile&&>);
+static_assert(!std::movable<int()>);
+static_assert(!std::movable<int (&)()>);
+static_assert(!std::movable<int[5]>);
+
+// Not move constructible or move assignable
+static_assert(!std::movable<mutex>);
+static_assert(!std::movable<std::optional<mutex> >);
+
+// T not move constructible
+struct no_move_constructor {
+  no_move_constructor(no_move_constructor&&) = delete;
+};
+static_assert(!std::movable<no_move_constructor>);
+
+// T& not assignable from T
+static_assert(!std::movable<int const>);
+static_assert(!std::movable<int const volatile>);
+
+struct no_move_assignment {
+  no_move_assignment& operator=(no_move_assignment&&) = delete;
+};
+static_assert(!std::movable<no_move_assignment>);
+
+struct mutable_copy_assignment {
+  mutable_copy_assignment& operator=(mutable_copy_assignment&);
+};
+static_assert(
+    !std::assignable_from<mutable_copy_assignment&, mutable_copy_assignment>);
+static_assert(!std::movable<mutable_copy_assignment>);
+
+struct const_move_constructor {
+  const_move_constructor& operator=(const_move_constructor&&) const;
+};
+static_assert(
+    !std::assignable_from<const_move_constructor&, const_move_constructor>);
+static_assert(!std::movable<const_move_constructor>);
+
+struct derived_from_nonmovable : mutex {};
+static_assert(!std::movable<derived_from_nonmovable>);
+
+struct has_a_nonmovable {
+  mutex m;
+};
+static_assert(!std::movable<has_a_nonmovable>);
+
+// `move_constructible and assignable_from<T&, T>` implies `swappable<T>`,
+// so there's nothing to test for the case of non-swappable.
+
+int main(int, char**) { return 0; }