[PATCH] D27565: [libcxx] Fix __compressed_pair so it doesn't copy the argument multiple times, and add constexpr.
Agustín Bergé via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 4 18:38:39 PDT 2017
K-ballo added a comment.
I don't see any potential layout change, other than the case in which both `T1` and `T2` are empty (which I understand doesn't happen within libc++). As far as I know, the rest of the cases should result in the same layout regardless of whether EBO actually applies or not.
Some notes inline.
================
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>
----------------
Should be `_IsEmpty`
================
Comment at: include/memory:2063
+ typedef typename remove_reference<_Tp>::type& reference;
+ typedef const typename remove_reference<_Tp>::type& const_reference;
----------------
If `_Tp` can actually be a reference type, turning it into reference to const here doesn't seem right
================
Comment at: include/memory:2110
+ _LIBCPP_CONSTEXPR explicit __compressed_pair_elem(_Up&& __u)
+ : __value_(_VSTD::forward<_Up>(__u)) {};
----------------
This use of `__value_` as a type is overly smart, consider just using `_Tp` for clarity
https://reviews.llvm.org/D27565
More information about the cfe-commits
mailing list