[libcxx-commits] [PATCH] D65260: [libc++] Consolidate swap, swap_ranges, and iter_swap in <type_traits>. NFC.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone created this revision.
Quuxplusone added reviewers: ldionne, EricWF, mclow.lists.
Herald added subscribers: libcxx-commits, dexonsmith.

Right now in master, `<type_traits>` contains the implementation of `swap(T&, T&)`, a forward-declaration of `swap(T(&)[N], T(&)[N])`, and the implementation of `iter_swap` in terms of ADL `swap`. Then, `<utility>` (which includes `<type_traits>`) contains the implementation of `swap(T(&)[N], T(&)[N])` in terms of `std::swap_ranges`, plus the implementation of `swap_ranges`.

This means that when the user's program does `#include <type_traits>`, they get a declaration but no definition for `swap(T(&)[N], T(&)[N])` — not technically wrong, not a bug, but mildly surprising. Also, this seems to mean <https://godbolt.org/z/TPkSF5> that changing the `enable_if` business on `swap(T(&)[N], T(&)[N])` would count as an ABI break (if only for user programs that fail to include-what-they-use and thus technically are non-conforming IIUC).

Most importantly, it means that when I want to modify `swap` (say, to add another overload specifically for trivially relocatable types), there are two headers to modify instead of just one. :)

After this patch, `<type_traits>` contains the implementation of `swap(T&, T&)`; the implementation of `swap(T(&)[N], T(&)[N])` in terms of `std::swap_ranges`; the implementation of `swap_ranges`; and the implementation of `iter_swap` in terms of ADL `swap`. It's a one-stop swap shop! Then `<utility>` simply includes `<type_traits>`, as before.

I don't intend this patch to have any observable effect for users.


Repository:
  rCXX libc++

https://reviews.llvm.org/D65260

Files:
  include/type_traits
  include/utility


Index: include/utility
===================================================================
--- include/utility
+++ include/utility
@@ -250,27 +250,13 @@
 
 // swap_ranges
 
+// moved to <type_traits> for better swap / noexcept support
 
-template <class _ForwardIterator1, class _ForwardIterator2>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
-_ForwardIterator2
-swap_ranges(_ForwardIterator1 __first1, _ForwardIterator1 __last1, _ForwardIterator2 __first2)
-{
-    for(; __first1 != __last1; ++__first1, (void) ++__first2)
-        swap(*__first1, *__first2);
-    return __first2;
-}
+// swap
 
-// forward declared in <type_traits>
-template<class _Tp, size_t _Np>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
-typename enable_if<
-    __is_swappable<_Tp>::value
->::type
-swap(_Tp (&__a)[_Np], _Tp (&__b)[_Np]) _NOEXCEPT_(__is_nothrow_swappable<_Tp>::value)
-{
-    _VSTD::swap_ranges(__a, __a + _Np, __b);
-}
+// moved to <type_traits> for better swap / noexcept support
+
+// move_if_noexcept
 
 template <class _Tp>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
Index: include/type_traits
===================================================================
--- include/type_traits
+++ include/type_traits
@@ -3681,6 +3681,13 @@
 template <class _Tp> struct __is_swappable;
 template <class _Tp> struct __is_nothrow_swappable;
 
+// swap, swap_ranges
+
+template <class _ForwardIterator1, class _ForwardIterator2>
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
+_ForwardIterator2
+swap_ranges(_ForwardIterator1 __first1, _ForwardIterator1 __last1, _ForwardIterator2 __first2);
+
 template <class _Tp>
 inline _LIBCPP_INLINE_VISIBILITY
 #ifndef _LIBCPP_CXX03_LANG
@@ -3706,7 +3713,22 @@
 typename enable_if<
     __is_swappable<_Tp>::value
 >::type
-swap(_Tp (&__a)[_Np], _Tp (&__b)[_Np]) _NOEXCEPT_(__is_nothrow_swappable<_Tp>::value);
+swap(_Tp (&__a)[_Np], _Tp (&__b)[_Np]) _NOEXCEPT_(__is_nothrow_swappable<_Tp>::value)
+{
+    _VSTD::swap_ranges(__a, __a + _Np, __b);
+}
+
+template <class _ForwardIterator1, class _ForwardIterator2>
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
+_ForwardIterator2
+swap_ranges(_ForwardIterator1 __first1, _ForwardIterator1 __last1, _ForwardIterator2 __first2)
+{
+    for(; __first1 != __last1; ++__first1, (void) ++__first2)
+        swap(*__first1, *__first2);
+    return __first2;
+}
+
+// iter_swap
 
 template <class _ForwardIterator1, class _ForwardIterator2>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65260.211657.patch
Type: text/x-patch
Size: 2567 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190725/d580fd3a/attachment-0001.bin>