[PATCH] D27565: [libcxx] Fix __compressed_pair so it doesn't copy the argument multiple times, and add constexpr.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked an inline comment as done.
EricWF added inline comments.


================
Comment at: include/memory:2058
 
-template <class _T1, class _T2, bool = is_same<typename remove_cv<_T1>::type,
-                                                     typename remove_cv<_T2>::type>::value,
-                                bool = is_empty<_T1>::value
-                                       && !__libcpp_is_final<_T1>::value,
-                                bool = is_empty<_T2>::value
-                                       && !__libcpp_is_final<_T2>::value
-         >
-struct __libcpp_compressed_pair_switch;
-
-template <class _T1, class _T2, bool IsSame>
-struct __libcpp_compressed_pair_switch<_T1, _T2, IsSame, false, false> {enum {value = 0};};
-
-template <class _T1, class _T2, bool IsSame>
-struct __libcpp_compressed_pair_switch<_T1, _T2, IsSame, true, false>  {enum {value = 1};};
-
-template <class _T1, class _T2, bool IsSame>
-struct __libcpp_compressed_pair_switch<_T1, _T2, IsSame, false, true>  {enum {value = 2};};
-
-template <class _T1, class _T2>
-struct __libcpp_compressed_pair_switch<_T1, _T2, false, true, true>    {enum {value = 3};};
-
-template <class _T1, class _T2>
-struct __libcpp_compressed_pair_switch<_T1, _T2, true, true, true>     {enum {value = 1};};
-
-template <class _T1, class _T2, unsigned = __libcpp_compressed_pair_switch<_T1, _T2>::value>
-class __libcpp_compressed_pair_imp;
+template <class _Tp, int _Idx, bool _IsSmall = is_empty<_Tp>::value
+                                       && !__libcpp_is_final<_Tp>::value>
----------------
K-ballo wrote:
> Should be `_IsEmpty`
Changed to `_CanBeEmptyBase`.


================
Comment at: include/memory:2063
+  typedef typename remove_reference<_Tp>::type& reference;
+  typedef const typename remove_reference<_Tp>::type& const_reference;
 
----------------
K-ballo wrote:
> If `_Tp` can actually be a reference type, turning it into reference to const here doesn't seem right
Agreed. Fixed by correctly collapsing `T&` -> `T&` instead of `T const&`


================
Comment at: include/memory:2110
+  _LIBCPP_CONSTEXPR explicit __compressed_pair_elem(_Up&& __u)
+      : __value_(_VSTD::forward<_Up>(__u)) {};
 
----------------
K-ballo wrote:
> This use of `__value_` as a type is overly smart, consider just using `_Tp` for clarity
Changed to `__value_type` instead.


https://reviews.llvm.org/D27565