[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

370

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 equality_comparable_with =
   equality_comparable<_Tp> && equality_comparable<_Up> &&
   common_reference_with<const remove_reference_t<_Tp>&, const remove_reference_t<_Up>&> &&
   equality_comparable<
     common_reference_t<
       const remove_reference_t<_Tp>&,
       const remove_reference_t<_Up>&>> &&
   __weakly_equality_comparable_with<_Tp, _Up>;
 
+// [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/lang/copyconstructible.compile.pass.cpp

 // UNSUPPORTED: libcpp-no-concepts
 
 // template<class T>
 // concept copy_constructible;
 
 #include <concepts>
 #include <type_traits>
 
-#include "moveconstructible.h"
+#include "type_classification/moveconstructible.h"
 
 // Tests in this namespace are shared with moveconstructible.pass.cpp
 // There are some interesting differences, so it's best if they're tested here
 // too.
 namespace MoveConstructibleTests {
 static_assert(std::copy_constructible<int>);
 static_assert(std::copy_constructible<int*>);
 static_assert(std::copy_constructible<int&>);

libcxx/test/std/concepts/lang/moveconstructible.compile.pass.cpp

 // UNSUPPORTED: libcpp-no-concepts
 
 // template<class T>
 // concept move_constructible;
 
 #include <concepts>
 #include <type_traits>
 
-#include "moveconstructible.h"
+#include "type_classification/moveconstructible.h"
 
 static_assert(std::move_constructible<int>);
 static_assert(std::move_constructible<int*>);
 static_assert(std::move_constructible<int&>);
 static_assert(std::move_constructible<int&&>);
 static_assert(std::move_constructible<const int>);
 static_assert(std::move_constructible<const int&>);
 static_assert(std::move_constructible<const int&&>);

 static_assert(std::move_constructible<DeletedMoveAssign&&>);
 static_assert(std::move_constructible<const DeletedMoveAssign&>);
 static_assert(std::move_constructible<const DeletedMoveAssign&&>);
 static_assert(std::move_constructible<ImplicitlyDeletedMoveAssign&>);
 static_assert(std::move_constructible<ImplicitlyDeletedMoveAssign&&>);
 static_assert(std::move_constructible<const ImplicitlyDeletedMoveAssign&>);
 static_assert(std::move_constructible<const ImplicitlyDeletedMoveAssign&&>);
 
+static_assert(!std::move_constructible<NonMovable>);
+static_assert(!std::move_constructible<DerivedFromNonMovable>);
+static_assert(!std::move_constructible<HasANonMovable>);
+
 int main(int, char**) { return 0; }

libcxx/test/std/concepts/lang/swappable.pass.cpp

 #include <map>
 #include <memory>
 #include <string>
 #include <optional>
 #include <unordered_map>
 #include <vector>
 
 #include "test_macros.h"
-#include "moveconstructible.h"
-#include "swappable.h"
+#include "type_classification/moveconstructible.h"
+#include "type_classification/swappable.h"
 
 template <class T>
 struct expected {
   T x;
   T y;
 };
 
 // clang-format off

libcxx/test/std/concepts/lang/swappable_with.compile.pass.cpp

 #include <map>
 #include <memory>
 #include <optional>
 #include <set>
 #include <unordered_map>
 #include <unordered_set>
 #include <vector>
 
-#include "moveconstructible.h"
-#include "swappable.h"
+#include "type_classification/moveconstructible.h"
+#include "type_classification/swappable.h"
 
 template <class T, class U>
 constexpr bool check_swappable_with_impl() {
   static_assert(std::swappable_with<T, U> == std::swappable_with<U, T>);
   return std::swappable_with<T, U>;
 }
 
 template <class T, class U>

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 T>
+// concept movable = see below;
+
+#include <concepts>
+
+#include <deque>
+#include <forward_list>
+#include <list>
+#include <map>
+#include <mutex>
+#include <optional>
+#include <unordered_map>
+#include <vector>
+
+#include "type_classification/moveconstructible.h"
+#include "type_classification/movable.h"
+
+// 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>);
+
+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::optional<std::vector<int> > >);
+static_assert(std::movable<std::vector<int> >);
+
+static_assert(std::movable<traditional_copy_assignment_only>);
+static_assert(std::movable<has_volatile_member>);
+static_assert(std::movable<has_array_member>);
+
+// 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]>);
+
+// Core non-move assignable.
+static_assert(!std::movable<int const>);
+static_assert(!std::movable<int const volatile>);
+
+static_assert(!std::movable<DeletedMoveCtor>);
+static_assert(!std::movable<ImplicitlyDeletedMoveCtor>);
+static_assert(!std::movable<DeletedMoveAssign>);
+static_assert(!std::movable<ImplicitlyDeletedMoveAssign>);
+static_assert(!std::movable<NonMovable>);
+static_assert(!std::movable<DerivedFromNonMovable>);
+static_assert(!std::movable<HasANonMovable>);
+
+static_assert(std::movable<cpp03_friendly>);
+static_assert(std::movable<const_move_ctor>);
+static_assert(std::movable<volatile_move_ctor>);
+static_assert(std::movable<cv_move_ctor>);
+static_assert(std::movable<multi_param_move_ctor>);
+static_assert(!std::movable<not_quite_multi_param_move_ctor>);
+
+static_assert(!std::assignable_from<copy_assign_with_mutable_parameter&,
+                                    copy_assign_with_mutable_parameter>);
+static_assert(!std::movable<copy_assign_with_mutable_parameter>);
+
+static_assert(!std::movable<const_move_assignment>);
+static_assert(std::movable<volatile_move_assignment>);
+static_assert(!std::movable<cv_move_assignment>);
+
+static_assert(!std::movable<const_move_assign_and_traditional_move_assign>);
+static_assert(!std::movable<volatile_move_assign_and_traditional_move_assign>);
+static_assert(!std::movable<cv_move_assign_and_traditional_move_assign>);
+static_assert(std::movable<const_move_assign_and_default_ops>);
+static_assert(std::movable<volatile_move_assign_and_default_ops>);
+static_assert(std::movable<cv_move_assign_and_default_ops>);
+
+static_assert(!std::movable<has_const_member>);
+static_assert(!std::movable<has_cv_member>);
+static_assert(!std::movable<has_lvalue_reference_member>);
+static_assert(!std::movable<has_rvalue_reference_member>);
+static_assert(!std::movable<has_function_ref_member>);
+
+static_assert(std::movable<deleted_assignment_from_const_rvalue>);
+
+// `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; }

libcxx/test/support/type_classification/movable.h

+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+#ifndef TEST_SUPPORT_TYPE_CLASSIFICATION_MOVABLE_H
+#define TEST_SUPPORT_TYPE_CLASSIFICATION_MOVABLE_H
+
+struct has_const_member {
+  int const x;
+};
+
+struct has_volatile_member {
+  int volatile x;
+};
+
+struct has_cv_member {
+  int const volatile x;
+};
+
+struct has_lvalue_reference_member {
+  int& x;
+};
+
+struct has_rvalue_reference_member {
+  int&& x;
+};
+
+struct has_array_member {
+  int x[5];
+};
+
+struct has_function_ref_member {
+  int (&f)();
+};
+
+struct cpp03_friendly {
+  cpp03_friendly(cpp03_friendly const&);
+  cpp03_friendly& operator=(cpp03_friendly const&);
+};
+
+struct const_move_ctor {
+  const_move_ctor(const_move_ctor const&&);
+  const_move_ctor& operator=(const_move_ctor&&);
+};
+
+struct volatile_move_ctor {
+  volatile_move_ctor(volatile_move_ctor volatile&&);
+  volatile_move_ctor& operator=(volatile_move_ctor&&);
+};
+
+struct cv_move_ctor {
+  cv_move_ctor(cv_move_ctor const volatile&&);
+  cv_move_ctor& operator=(cv_move_ctor&&);
+};
+
+struct multi_param_move_ctor {
+  multi_param_move_ctor(multi_param_move_ctor&&, int = 0);
+  multi_param_move_ctor& operator=(multi_param_move_ctor&&);
+};
+
+struct not_quite_multi_param_move_ctor {
+  not_quite_multi_param_move_ctor(not_quite_multi_param_move_ctor&&, int);
+  not_quite_multi_param_move_ctor& operator=(not_quite_multi_param_move_ctor&&);
+};
+
+struct traditional_copy_assignment_only {
+  traditional_copy_assignment_only&
+  operator=(traditional_copy_assignment_only const&);
+};
+
+struct copy_assign_with_mutable_parameter {
+  copy_assign_with_mutable_parameter&
+  operator=(copy_assign_with_mutable_parameter&);
+};
+
+struct const_move_assignment {
+  const_move_assignment& operator=(const_move_assignment&&) const;
+};
+
+struct volatile_move_assignment {
+  const_move_assignment& operator=(const_move_assignment&&) volatile;
+};
+
+struct cv_move_assignment {
+  cv_move_assignment& operator=(cv_move_assignment&&) const volatile;
+};
+
+struct const_move_assign_and_traditional_move_assign {
+  const_move_assign_and_traditional_move_assign&
+  operator=(const_move_assign_and_traditional_move_assign&&);
+  const_move_assign_and_traditional_move_assign&
+  operator=(const_move_assign_and_traditional_move_assign&&) const;
+};
+
+struct volatile_move_assign_and_traditional_move_assign {
+  volatile_move_assign_and_traditional_move_assign&
+  operator=(volatile_move_assign_and_traditional_move_assign&&);
+  volatile_move_assign_and_traditional_move_assign&
+  operator=(volatile_move_assign_and_traditional_move_assign&&) volatile;
+};
+
+struct cv_move_assign_and_traditional_move_assign {
+  cv_move_assign_and_traditional_move_assign&
+  operator=(cv_move_assign_and_traditional_move_assign&&);
+  cv_move_assign_and_traditional_move_assign&
+  operator=(cv_move_assign_and_traditional_move_assign&&) const volatile;
+};
+
+struct const_move_assign_and_default_ops {
+  const_move_assign_and_default_ops(const_move_assign_and_default_ops const&) =
+      default;
+  const_move_assign_and_default_ops(const_move_assign_and_default_ops&&) =
+      default;
+  const_move_assign_and_default_ops&
+  operator=(const_move_assign_and_default_ops const&) = default;
+  const_move_assign_and_default_ops&
+  operator=(const_move_assign_and_default_ops&&) = default;
+  const_move_assign_and_default_ops&
+  operator=(const_move_assign_and_default_ops&&) const;
+};
+
+struct volatile_move_assign_and_default_ops {
+  volatile_move_assign_and_default_ops(
+      volatile_move_assign_and_default_ops const&) = default;
+  volatile_move_assign_and_default_ops(volatile_move_assign_and_default_ops&&) =
+      default;
+  volatile_move_assign_and_default_ops&
+  operator=(volatile_move_assign_and_default_ops const&) = default;
+  volatile_move_assign_and_default_ops&
+  operator=(volatile_move_assign_and_default_ops&&) = default;
+  volatile_move_assign_and_default_ops&
+  operator=(volatile_move_assign_and_default_ops&&) volatile;
+};
+
+struct cv_move_assign_and_default_ops {
+  cv_move_assign_and_default_ops(cv_move_assign_and_default_ops const&) =
+      default;
+  cv_move_assign_and_default_ops(cv_move_assign_and_default_ops&&) = default;
+  cv_move_assign_and_default_ops&
+  operator=(cv_move_assign_and_default_ops const&) = default;
+  cv_move_assign_and_default_ops&
+  operator=(cv_move_assign_and_default_ops&&) = default;
+  cv_move_assign_and_default_ops&
+  operator=(cv_move_assign_and_default_ops&&) const volatile;
+};
+
+struct deleted_assignment_from_const_rvalue {
+  deleted_assignment_from_const_rvalue(
+      deleted_assignment_from_const_rvalue const&);
+  deleted_assignment_from_const_rvalue(deleted_assignment_from_const_rvalue&&);
+  deleted_assignment_from_const_rvalue&
+  operator=(const deleted_assignment_from_const_rvalue&);
+  deleted_assignment_from_const_rvalue&
+  operator=(deleted_assignment_from_const_rvalue&&);
+  deleted_assignment_from_const_rvalue&
+  operator=(const deleted_assignment_from_const_rvalue&&) = delete;
+};
+
+#endif // TEST_SUPPORT_TYPE_CLASSIFICATION_MOVABLE_H

libcxx/test/support/type_classification/moveconstructible.h

 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
-#ifndef TEST_STD_CONCEPTS_LAND_MOVECONSTRUCTIBLE_H
-#define TEST_STD_CONCEPTS_LAND_MOVECONSTRUCTIBLE_H
+#ifndef TEST_SUPPORT_TYPE_CLASSIFICATION_MOVECONSTRUCTIBLE_H
+#define TEST_SUPPORT_TYPE_CLASSIFICATION_MOVECONSTRUCTIBLE_H
 
 struct HasDefaultOps {};
 
 struct CustomMoveCtor {
   CustomMoveCtor(CustomMoveCtor&&) noexcept;
 };
 
 struct MoveOnly {

 };
 
 struct DerivedFromNonMovable : NonMovable {};
 
 struct HasANonMovable {
   NonMovable X;
 };
 
-#endif // TEST_STD_CONCEPTS_LAND_MOVECONSTRUCTIBLE_H
+#endif // TEST_SUPPORT_TYPE_CLASSIFICATION_MOVECONSTRUCTIBLE_H

libcxx/test/support/type_classification/swappable.h

 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
-#ifndef TEST_STD_CONCEPTS_LANG_SWAPPABLE_H
-#define TEST_STD_CONCEPTS_LANG_SWAPPABLE_H
+#ifndef TEST_SUPPORT_TYPE_CLASSIFICATION_SWAPPABLE_H
+#define TEST_SUPPORT_TYPE_CLASSIFICATION_SWAPPABLE_H
 
 #include <concepts>
 
 // `adl_swappable` indicates it's been swapped using ADL by maintaining a pointer to itself that
 // isn't a part of the exchange. This is well-formed since we say that two `adl_swappable` objects
 // are equal only if their respective `value` subobjects are equal and their respective `this`
 // subobjects store the addresses of those respective `adl_swappable` objects.
 class lvalue_adl_swappable {
operator==(throwable_adl_swappable const& other) const noexcept {
     return value_ == other.value_ && this_ == this && other.this_ == &other;
   }
 
 private:
   int value_{};
   throwable_adl_swappable* this_ = this;
 };
 
-#endif // TEST_STD_CONCEPTS_LANG_SWAPPABLE_H
+#endif // TEST_SUPPORT_TYPE_CLASSIFICATION_SWAPPABLE_H