Properly constrain _Movable_box's copy constructors (#3171)

Co-authored-by: Stephan T. Lavavej <[email protected]> Co-authored-by: Jakub Mazurkiewicz <[email protected]>
microsoft · Oct 26, 2022 · 705265e · 705265e
1 parent c1c21e9
commit 705265e
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 17 deletions.
diff --git a/stl/inc/iterator b/stl/inc/iterator
@@ -571,14 +571,14 @@ public:
         : _First(_STD forward<_Types>(_Args)...), _Contains{_Variantish_state::_Holds_first} {}
 
     template <class... _Types>
-        requires _Not_same_as<_Ty1, _Ty2>
+        requires _Different_from<_Ty1, _Ty2>
     constexpr explicit _Variantish(in_place_type_t<_Ty2>, _Types&&... _Args) noexcept(
         is_nothrow_constructible_v<_Ty2, _Types...>)
         : _Second(_STD forward<_Types>(_Args)...), _Contains{_Variantish_state::_Holds_second} {}
 
     // clang-format off
     template <class _Uty1, class _Uty2>
-        requires _Not_same_as<_Variantish<_Uty1, _Uty2>, _Variantish>
+        requires _Different_from<_Variantish<_Uty1, _Uty2>, _Variantish>
     constexpr _Variantish(const _Variantish<_Uty1, _Uty2>& _That) noexcept(
         is_nothrow_constructible_v<_Ty1, const _Uty1&> && is_nothrow_constructible_v<_Ty2, const _Uty2&>)
         : _Contains{_That._Contains} {
@@ -734,7 +734,7 @@ public:
 
     // clang-format off
     template <class _Uty1, class _Uty2>
-        requires _Not_same_as<_Variantish<_Uty1, _Uty2>, _Variantish>
+        requires _Different_from<_Variantish<_Uty1, _Uty2>, _Variantish>
     constexpr _Variantish& operator=(const _Variantish<_Uty1, _Uty2>& _That) noexcept(
         is_nothrow_constructible_v<_Ty1, const _Uty1&> && is_nothrow_constructible_v<_Ty2, const _Uty2&>
             && is_nothrow_assignable_v<_Ty1&, const _Uty1&> && is_nothrow_assignable_v<_Ty2&, const _Uty2&>) {

diff --git a/stl/inc/ranges b/stl/inc/ranges
@@ -269,10 +269,13 @@ namespace ranges {
         // clang-format off
         ~_Movable_box() requires is_trivially_destructible_v<_Ty> = default;
 
-        _Movable_box(const _Movable_box&) requires is_trivially_copy_constructible_v<_Ty> = default;
+        _Movable_box(const _Movable_box&)
+            requires copy_constructible<_Ty> && is_trivially_copy_constructible_v<_Ty> = default;
         // clang-format on
 
-        constexpr _Movable_box(const _Movable_box& _That) : _Engaged{_That._Engaged} {
+        constexpr _Movable_box(const _Movable_box& _That)
+            requires copy_constructible<_Ty>
+            : _Engaged{_That._Engaged} {
             if (_That._Engaged) {
                 _Construct_in_place(_Val, _That._Val);
             }
@@ -506,15 +509,15 @@ namespace ranges {
             }
         }
 
-        template <_Not_same_as<_Ty> _Uty>
+        template <_Different_from<_Ty> _Uty>
             requires convertible_to<const _Uty&, _Ty>
         constexpr _Defaultabox(const _Defaultabox<_Uty>& _That) : _Engaged{_That} {
             if (_That) {
                 _Construct_in_place(_Val, *_That);
             }
         }
 
-        template <_Not_same_as<_Ty> _Uty>
+        template <_Different_from<_Ty> _Uty>
             requires convertible_to<_Uty, _Ty>
         constexpr _Defaultabox(_Defaultabox<_Uty>&& _That) : _Engaged{_That} {
             if (_That) {
@@ -639,15 +642,15 @@ namespace ranges {
     public:
         _Defaultabox() = default;
 
-        template <_Not_same_as<_Ty> _Uty>
+        template <_Different_from<_Ty> _Uty>
             requires convertible_to<const _Uty&, _Ty>
         constexpr _Defaultabox(const _Defaultabox<_Uty>& _That) {
             if (_That) {
                 _Value = static_cast<_Ty>(*_That);
             }
         }
 
-        template <_Not_same_as<_Ty> _Uty>
+        template <_Different_from<_Ty> _Uty>
             requires convertible_to<_Uty, _Ty>
         constexpr _Defaultabox(_Defaultabox<_Uty>&& _That) {
             if (_That) {
@@ -1481,7 +1484,7 @@ namespace ranges {
 
     public:
         // clang-format off
-        template <_Not_same_as<ref_view> _OtherRng>
+        template <_Different_from<ref_view> _OtherRng>
         constexpr ref_view(_OtherRng&& _Other) noexcept(
             noexcept(static_cast<_Rng&>(_STD forward<_OtherRng>(_Other)))) // strengthened
             requires convertible_to<_OtherRng, _Rng&> && requires {

diff --git a/stl/inc/xutility b/stl/inc/xutility
@@ -1732,7 +1732,7 @@ _NODISCARD constexpr const _Elem* data(initializer_list<_Elem> _Ilist) noexcept
 
 #ifdef __cpp_lib_concepts
 template <class _Ty1, class _Ty2>
-concept _Not_same_as = (!same_as<remove_cvref_t<_Ty1>, remove_cvref_t<_Ty2>>);
+concept _Different_from = (!same_as<remove_cvref_t<_Ty1>, remove_cvref_t<_Ty2>>);
 
 #if _HAS_CXX23
 _EXPORT_STD template <indirectly_readable _Ty>
@@ -1775,8 +1775,8 @@ struct _Basic_const_iterator_category<_Iter> {
 
 // TRANSITION, LLVM-55945: These are distinct concepts as a workaround
 template <class _Ty, class _Iter>
-concept _Bci_order =
-    _Not_same_as<_Ty, basic_const_iterator<_Iter>> && random_access_iterator<_Iter> && totally_ordered_with<_Iter, _Ty>;
+concept _Bci_order = _Different_from<_Ty, basic_const_iterator<_Iter>> && random_access_iterator<_Iter>
+                  && totally_ordered_with<_Iter, _Ty>;
 
 template <class _Ty, class _Iter>
 concept _Bci_order_3way = _Bci_order<_Ty, _Iter> && three_way_comparable_with<_Iter, _Ty>;
@@ -1823,7 +1823,7 @@ public:
         is_nothrow_constructible_v<_Iter, _Other>) // strengthened
         : _Current(_STD move(_Current_._Current)) {}
 
-    template <_Not_same_as<basic_const_iterator> _Other>
+    template <_Different_from<basic_const_iterator> _Other>
         requires convertible_to<_Other, _Iter>
     constexpr basic_const_iterator(_Other&& _Current_) noexcept(
         is_nothrow_constructible_v<_Iter, _Other>) // strengthened
@@ -3509,7 +3509,7 @@ namespace ranges {
             }
         }
 
-        template <_Not_same_as<subrange> _Rng>
+        template <_Different_from<subrange> _Rng>
             requires borrowed_range<_Rng>
                 && _Convertible_to_non_slicing<iterator_t<_Rng>, _It>
                 && convertible_to<sentinel_t<_Rng>, _Se>
@@ -3522,7 +3522,7 @@ namespace ranges {
             : subrange{_RANGES begin(_Val), _RANGES end(_Val), _Count} {}
         // clang-format on
 
-        template <_Not_same_as<subrange> _Pair_like>
+        template <_Different_from<subrange> _Pair_like>
             requires _Pair_like_convertible_from<_Pair_like, const _It&, const _Se&>
         constexpr operator _Pair_like() const {
             return _Pair_like(_First, _Last);

diff --git a/tests/std/tests/P2494R2_move_only_range_adaptors/test.cpp b/tests/std/tests/P2494R2_move_only_range_adaptors/test.cpp
@@ -58,10 +58,24 @@ void test_transform() {
                       : movable<T> || is_nothrow_move_constructible_v<T>) {
         static_assert(sizeof(ranges::_Movable_box<T>) == sizeof(T));
     } else {
-        static_assert(sizeof(ranges::_Movable_box<T>) > sizeof(T));
+        static_assert(sizeof(ranges::_Movable_box<T>) == sizeof(T) + alignof(T));
     }
 }
 
+// Test that _Movable_box's copy operations are properly constrained
+template <bool CanDefault>
+struct move_construct_only {
+    move_construct_only()
+        requires CanDefault;
+    move_construct_only(move_construct_only&&);
+};
+static_assert(copy_constructible<ranges::_Movable_box<int>>);
+static_assert(copyable<ranges::_Movable_box<int>>);
+static_assert(!copy_constructible<ranges::_Movable_box<move_construct_only<true>>>);
+static_assert(!copyable<ranges::_Movable_box<move_construct_only<true>>>);
+static_assert(!copy_constructible<ranges::_Movable_box<move_construct_only<false>>>);
+static_assert(!copyable<ranges::_Movable_box<move_construct_only<false>>>);
+
 int main() {
     test_transform<Nothrow, Nothrow, Nothrow, Nothrow>();
     test_transform<Throwing, Throwing, Throwing, Throwing>();