[libc++][NFC] Reformat and modernize compressed_pair.h

[ldionne]

LGTM, I looked at everything and this is indeed NFC. I don't understand why this is failing in C++03 mode, either.

Ship it but please consider my comments.

libcxx/include/__memory/compressed_pair.h

68

Suggestion: move the explicit to the next line, otherwise it's really easy to miss.

119

It's more customary to leave the attributes on the line above, like so:

_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
typename _Base1::reference first() _NOEXCEPT { ... }

Otherwise, it's difficult to parse the function declaration itself because it's too obscured by these annotations. I think it's fine to leave the constructors above like _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __compressed_pair_elem() = default; as-is since it allows aligning them and their body is empty anyway, so that's more readable.

[Mordante]

Nice cleaups! LGTM modulo some style nits and the C++03 breakage..

libcxx/include/__memory/compressed_pair.h

33

Same on line 31.
Just curious did you reformat manually or using clang-format?

38

Needed for C++03.

61

67

[philnik]

• Address comments

libcxx/include/__memory/compressed_pair.h

33

I re-formatted manually.

[• Quuxplusone]

Looks acceptable to me; I'm not sure it buys us anything, but it looks harmless.

libcxx/include/__memory/compressed_pair.h

68

+1, explicit should come on the same line (and immediately preceding) the ctor's name.
I'd like to see explicit added to the ctors on new lines 63, 64, 65, and 74 as well, or know the reason why that would break things. Implicit conversions are the devil, and we shouldn't need any non-explicit ctors anywhere on the implementation side of things. (As opposed to on the public-API side, like std::pair and std::vector and so on, where the Standard mandates many ctors to be non-explicit.)

94–95

If Clang is happy with this, probable yikes. git grep tells me that the majority position is pre-=:

using _Base1 _LIBCPP_NODEBUG = __compressed_pair_elem<_T1, 0>;

and I'd be worried that post-= meant something different to the compiler.

108

FWIW, I'd prefer to leave this : ... on the next line, for clarity and to match line 115 below.

[philnik]

• Address comments

libcxx/include/__memory/compressed_pair.h

68

I want this to stay NFC, so I won't add explicit here.

libcxx/include/__memory/compressed_pair.h

 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 // Tag used to default initialize one or both of the pair's elements.
 struct __default_init_tag {};
 struct __value_init_tag {};
 
-template <class _Tp, int _Idx,
-          bool _CanBeEmptyBase =
-              is_empty<_Tp>::value && !__libcpp_is_final<_Tp>::value>
+template <class _Tp, int _Idx, bool _CanBeEmptyBase = is_empty<_Tp>::value && !__libcpp_is_final<_Tp>::value>
 struct __compressed_pair_elem {
-  typedef _Tp _ParamT;
-  typedef _Tp& reference;
-  typedef const _Tp& const_reference;
-
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-  __compressed_pair_elem(__default_init_tag) {}
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-  __compressed_pair_elem(__value_init_tag) : __value_() {}
-
-  template <class _Up, class = typename enable_if<
-      !is_same<__compressed_pair_elem, typename decay<_Up>::type>::value
-  >::type>
-  _LIBCPP_INLINE_VISIBILITY
-  _LIBCPP_CONSTEXPR explicit
-  __compressed_pair_elem(_Up&& __u)
-      : __value_(_VSTD::forward<_Up>(__u))
-    {
-    }
-
+  using _ParamT = _Tp ;
+  using reference = _Tp&;
+  using const_reference = const _Tp& ;
+
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __compressed_pair_elem(__default_init_tag) {}
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __compressed_pair_elem(__value_init_tag) : __value_() {}
+
+  template <class _Up, class = __enable_if_t<!is_same<__compressed_pair_elem, typename decay<_Up>::type>::value>>
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR explicit
+  __compressed_pair_elem(_Up&& __u) : __value_(std::forward<_Up>(__u)) {}
 
 #ifndef _LIBCPP_CXX03_LANG
-  template <class... _Args, size_t... _Indexes>
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-  __compressed_pair_elem(piecewise_construct_t, tuple<_Args...> __args,
-                         __tuple_indices<_Indexes...>)
-      : __value_(_VSTD::forward<_Args>(_VSTD::get<_Indexes>(__args))...) {}
+  template <class... _Args, size_t... _Indices>
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX14
+  __compressed_pair_elem(piecewise_construct_t, tuple<_Args...> __args, __tuple_indices<_Indices...>)
+      : __value_(std::forward<_Args>(std::get<_Indices>(__args))...) {}
 #endif
 
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 reference __get() _NOEXCEPT { return __value_; }
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR const_reference __get() const _NOEXCEPT { return __value_; }
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11 reference __get() _NOEXCEPT { return __value_; }
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR const_reference __get() const _NOEXCEPT { return __value_; }
 
 private:
   _Tp __value_;
 };
 
 template <class _Tp, int _Idx>
 struct __compressed_pair_elem<_Tp, _Idx, true> : private _Tp {
-  typedef _Tp _ParamT;
-  typedef _Tp& reference;
-  typedef const _Tp& const_reference;
-  typedef _Tp __value_type;
-
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR __compressed_pair_elem() = default;
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-  __compressed_pair_elem(__default_init_tag) {}
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-  __compressed_pair_elem(__value_init_tag) : __value_type() {}
-
-  template <class _Up, class = typename enable_if<
-        !is_same<__compressed_pair_elem, typename decay<_Up>::type>::value
-  >::type>
-  _LIBCPP_INLINE_VISIBILITY
-  _LIBCPP_CONSTEXPR explicit
-  __compressed_pair_elem(_Up&& __u)
-      : __value_type(_VSTD::forward<_Up>(__u))
-  {}
+  using _ParamT = _Tp;
+  using reference = _Tp&;
+  using const_reference = const _Tp&;
+  using __value_type = _Tp ;
+
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __compressed_pair_elem() = default;
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __compressed_pair_elem(__default_init_tag) {}
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __compressed_pair_elem(__value_init_tag) : __value_type() {}
+
+  template <class _Up, class = __enable_if_t<!is_same<__compressed_pair_elem, typename decay<_Up>::type>::value>>
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR explicit
+  __compressed_pair_elem(_Up&& __u) : __value_type(std::forward<_Up>(__u)) {}
 
 #ifndef _LIBCPP_CXX03_LANG
-  template <class... _Args, size_t... _Indexes>
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-  __compressed_pair_elem(piecewise_construct_t, tuple<_Args...> __args,
-                         __tuple_indices<_Indexes...>)
-      : __value_type(_VSTD::forward<_Args>(_VSTD::get<_Indexes>(__args))...) {}
+  template <class... _Args, size_t... _Indices>
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX14
+  __compressed_pair_elem(piecewise_construct_t, tuple<_Args...> __args, __tuple_indices<_Indices...>)
+      : __value_type(std::forward<_Args>(std::get<_Indices>(__args))...) {}
 #endif
 
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 reference __get() _NOEXCEPT { return *this; }
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR const_reference __get() const _NOEXCEPT { return *this; }
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11 reference __get() _NOEXCEPT { return *this; }
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR const_reference __get() const _NOEXCEPT { return *this; }
 };
 
 template <class _T1, class _T2>
 class __compressed_pair : private __compressed_pair_elem<_T1, 0>,
                           private __compressed_pair_elem<_T2, 1> {
 public:
   // NOTE: This static assert should never fire because __compressed_pair
   // is *almost never* used in a scenario where it's possible for T1 == T2.
   // (The exception is std::function where it is possible that the function
   //  object and the allocator have the same type).
   static_assert((!is_same<_T1, _T2>::value),
     "__compressed_pair cannot be instantiated when T1 and T2 are the same type; "
-    "The current implementation is NOT ABI-compatible with the previous "
-    "implementation for this configuration");
+    "The current implementation is NOT ABI-compatible with the previous implementation for this configuration");
 
-    typedef _LIBCPP_NODEBUG __compressed_pair_elem<_T1, 0> _Base1;
-    typedef _LIBCPP_NODEBUG __compressed_pair_elem<_T2, 1> _Base2;
+    using _Base1 = _LIBCPP_NODEBUG __compressed_pair_elem<_T1, 0>;
+    using _Base2 = _LIBCPP_NODEBUG __compressed_pair_elem<_T2, 1>;
 
     template <bool _Dummy = true,
-      class = typename enable_if<
+      class = __enable_if_t<
           __dependent_type<is_default_constructible<_T1>, _Dummy>::value &&
           __dependent_type<is_default_constructible<_T2>, _Dummy>::value
-      >::type
       >
-  _LIBCPP_INLINE_VISIBILITY
-  _LIBCPP_CONSTEXPR __compressed_pair() : _Base1(__value_init_tag()), _Base2(__value_init_tag()) {}
+  >
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
+  __compressed_pair() : _Base1(__value_init_tag()), _Base2(__value_init_tag()) {}
 
   template <class _U1, class _U2>
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-  __compressed_pair(_U1&& __t1, _U2&& __t2)
-      : _Base1(_VSTD::forward<_U1>(__t1)), _Base2(_VSTD::forward<_U2>(__t2)) {}
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
+  __compressed_pair(_U1&& __t1, _U2&& __t2) : _Base1(std::forward<_U1>(__t1)), _Base2(std::forward<_U2>(__t2)) {}
 
 #ifndef _LIBCPP_CXX03_LANG
   template <class... _Args1, class... _Args2>
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX14
   __compressed_pair(piecewise_construct_t __pc, tuple<_Args1...> __first_args,
                     tuple<_Args2...> __second_args)
-      : _Base1(__pc, _VSTD::move(__first_args),
-               typename __make_tuple_indices<sizeof...(_Args1)>::type()),
-        _Base2(__pc, _VSTD::move(__second_args),
-               typename __make_tuple_indices<sizeof...(_Args2)>::type()) {}
+      : _Base1(__pc, std::move(__first_args), typename __make_tuple_indices<sizeof...(_Args1)>::type()),
+        _Base2(__pc, std::move(__second_args), typename __make_tuple_indices<sizeof...(_Args2)>::type()) {}
 #endif
 
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 typename _Base1::reference first() _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11 typename _Base1::reference first() _NOEXCEPT {
     return static_cast<_Base1&>(*this).__get();
   }
 
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR typename _Base1::const_reference first() const _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR typename _Base1::const_reference first() const _NOEXCEPT {
     return static_cast<_Base1 const&>(*this).__get();
   }
 
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 typename _Base2::reference second() _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11 typename _Base2::reference second() _NOEXCEPT {
     return static_cast<_Base2&>(*this).__get();
   }
 
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR typename _Base2::const_reference second() const _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR typename _Base2::const_reference second() const _NOEXCEPT {
     return static_cast<_Base2 const&>(*this).__get();
   }
 
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-  static _Base1* __get_first_base(__compressed_pair* __pair) _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR static _Base1* __get_first_base(__compressed_pair* __pair) _NOEXCEPT {
     return static_cast<_Base1*>(__pair);
   }
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-  static _Base2* __get_second_base(__compressed_pair* __pair) _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR static _Base2* __get_second_base(__compressed_pair* __pair) _NOEXCEPT {
     return static_cast<_Base2*>(__pair);
   }
 
-  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 void swap(__compressed_pair& __x)
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11 void swap(__compressed_pair& __x)
       _NOEXCEPT_(__is_nothrow_swappable<_T1>::value && __is_nothrow_swappable<_T2>::value) {
-    using _VSTD::swap;
+    using std::swap;
     swap(first(), __x.first());
     swap(second(), __x.second());
   }
 };
 
 template <class _T1, class _T2>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
 void swap(__compressed_pair<_T1, _T2>& __x, __compressed_pair<_T1, _T2>& __y)
     _NOEXCEPT_(__is_nothrow_swappable<_T1>::value && __is_nothrow_swappable<_T2>::value) {
   __x.swap(__y);
 }
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___MEMORY_COMPRESSED_PAIR_H