[libcxx] replaces `__compressed_pair` with `[[no_unique_address]]` in `unique_ptr`

[cjdb]

Summary

__compressed_pair is a pre-[[no_unique_address]] way to ensure that
empty classes don't unnecessarily increase object sizes. It also
unfortunately bloats program sizes in debug builds due to added program
information. This commit replaces __compressed_pair with the
attribute, which reduces debug build sizes.

Below is a size benchmark of Clang built in debug mode with and without
__compressed_pair, using bloaty.

   FILE SIZE        VM SIZE
--------------  --------------
 +0.0% +1.94Ki  +0.0% +1.94Ki    .rodata
 +0.0%    +248  +0.0%    +248    .dynstr
 +0.2%     +28  [ = ]       0    .debug_loclists
 -0.5%      -1  [ = ]       0    .debug_aranges
 [DEL]     -56  [DEL]     -56    [LOAD #2 [R]]
 -0.2% -12.5Ki  [ = ]       0    .debug_abbrev
 -1.3%  -176Ki  [ = ]       0    .debug_rnglists
 -2.5%  -207Ki  -2.5%  -207Ki    .eh_frame_hdr
 -1.2%  -409Ki  [ = ]       0    .debug_addr
 -0.8%  -523Ki  [ = ]       0    .debug_str_offsets
 -2.5%  -622Ki  [ = ]       0    .symtab
 -2.5%  -830Ki  -2.5%  -830Ki    .eh_frame
 -0.7% -1.06Mi  -0.7% -1.06Mi    .text
 -1.0% -1.51Mi  [ = ]       0    .debug_line
 -3.2% -4.24Mi  [ = ]       0    .strtab
 -1.5% -5.99Mi  [ = ]       0    .debug_info
 -3.8% -10.3Mi  [ = ]       0    .debug_str
 -1.9% -25.8Mi  -0.9% -2.07Mi    TOTAL

[philnik]

As-is this is an ABI break. Your unique_ptr<int> is currently 16 bytes large: https://godbolt.org/z/9xWPjTzK3. A lot of the reduced symbols is probably the avoidance of __compressed_pair right now. That is most likely possible to do in an ABI-stable way, but we need no_unique_address to work on windows for that to make sense. I'm also not convinced that duplicating the entire class is a good idea in terms of maintainability.

[dblaikie]

In D153991#4456931, @philnik wrote:

A lot of the reduced symbols is probably the avoidance of __compressed_pair right now.

On that note - spun off this discussion: https://discourse.llvm.org/t/reducing-overhead-of-compressed-pair/71660

[EricWF]

In D153991#4457035, @dblaikie wrote:

In D153991#4456931, @philnik wrote:

A lot of the reduced symbols is probably the avoidance of __compressed_pair right now.

On that note - spun off this discussion: https://discourse.llvm.org/t/reducing-overhead-of-compressed-pair/71660

Indeed. compressed pairs symbols have been a pain in our ass forever. That's a problem worth tackling , but I'm not sure it's trivial to do that.
As Philinik say, no_unique_address is a bit of a problem. But I disagree that we need it on Windows in order proceed with changes.

IMHO our user base on Windows is so small that pessimizing Windows to optimize for the masses makes sense.

[ldionne]

libcxx/include/__memory/unique_ptr.h

710–712

I believe that makes us non-conforming? get_deleter() returns a reference to that, so different deleters need to have different addresses. It's a bit pedantic for sure, but this still seems like not what the standard intended us to implement unique_ptr as.

If we get the same (or almost the same) benefits from simply avoiding __compressed_pair, I would personally prefer that approach.

[cjdb]

libcxx/include/__memory/unique_ptr.h

710–712

The wording says that it needs to return a reference: it doesn't say that it needs to return references to unique objects. Given that this is also for a stateless allocator, I don't think it's non-conforming at all.

Note that I'm pretty committed to trying to get rid of __compressed_pair, since that improves the situation for everyone (but happy to debate the conformance matter above, here or elsewhere).

[cjdb]

updates patch to be about removing __compressed_pair instead of specialising the default_delete case

[cjdb]

slight update to the commit message

[EricWF]

It's my understanding that the clang-cl changes to support [[no_unique_address]] are coming down the pipe, and will hopefully be here within the quarter.
I think we should wait until that lands, and then proceed carefully.

There's still some potentially breaking ABI changes like final but I'm open to erroring on that or supporting it minimally.

libcxx/include/__memory/unique_ptr.h

509

Shouldn't this follow the swap protocol using std::swap.

[Mordante]

In D153991#4558771, @EricWF wrote:

It's my understanding that the clang-cl changes to support [[no_unique_address]] are coming down the pipe, and will hopefully be here within the quarter.

@EricWF do you have information when that clang-cl patch will land? We're moving to GitHub PRs when the patch does not land soon it might be best to abandon this patch and open a GitHub PR instead. @cjdb if you open a PR can you make sure you address all open comments on this patch?

[akhuang]

I'm working on a clang-cl no_unique_address patch here: https://github.com/llvm/llvm-project/pull/65675

libcxx/include/__config

 //
 // The incorrect implementation of CityHash has the problem that it drops some
 // bits on the floor.
 #    define _LIBCPP_ABI_FIX_CITYHASH_IMPLEMENTATION
 // Remove the base 10 implementation of std::to_chars from the dylib.
 // The implementation moved to the header, but we still export the symbols from
 // the dylib for backwards compatibility.
 #    define _LIBCPP_ABI_DO_NOT_EXPORT_TO_CHARS_BASE_10
+
+// Defines exactly one std::default_delete<T> for the whole program, as opposed
+// to one per std::unique_ptr<T> (only relevant to std::unique_ptr: other instances
+// can still exist).
+#    define _LIBCPP_EXACTLY_ONE_DEFAULT_DELETE_PER_TYPE
 #  elif _LIBCPP_ABI_VERSION == 1
 #    if !(defined(_LIBCPP_OBJECT_FORMAT_COFF) || defined(_LIBCPP_OBJECT_FORMAT_XCOFF))
 // Enable compiling copies of now inline methods into the dylib to support
 // applications compiled against older libraries. This is unnecessary with
 // COFF dllexport semantics, since dllexport forces a non-inline definition
 // of inline functions to be emitted anyway. Our own non-inline copy would
 // conflict with the dllexport-emitted copy, so we disable it. For XCOFF,
 // the linker will take issue with the symbols in the shared object if the

libcxx/include/__memory/unique_ptr.h

 #include <__type_traits/is_default_constructible.h>
 #include <__type_traits/is_function.h>
 #include <__type_traits/is_pointer.h>
 #include <__type_traits/is_reference.h>
 #include <__type_traits/is_same.h>
 #include <__type_traits/is_swappable.h>
 #include <__type_traits/is_void.h>
 #include <__type_traits/remove_extent.h>
+#include <__type_traits/remove_pointer.h>
 #include <__type_traits/type_identity.h>
+#include <__utility/exchange.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
 #include <cstddef>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
if (__tmp)
       __ptr_.second()(__tmp);
   }
 
   _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 void swap(unique_ptr& __u) _NOEXCEPT {
     __ptr_.swap(__u.__ptr_);
   }
 };
 
-
 template <class _Tp, class _Dp>
 class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp> {
 public:
   typedef _Tp element_type;
   typedef _Dp deleter_type;
   typedef typename __pointer<_Tp, deleter_type>::type pointer;
 
 private:
public:
   _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 void reset(nullptr_t = nullptr) _NOEXCEPT {
     pointer __tmp = __ptr_.first();
     __ptr_.first() = nullptr;
     if (__tmp)
       __ptr_.second()(__tmp);
   }
 
   _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 void swap(unique_ptr& __u) _NOEXCEPT {
     __ptr_.swap(__u.__ptr_);
   }
 };
 
+template <class _Tp>
+class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp, default_delete<_Tp>> {
+public:
+  using pointer _LIBCPP_NODEBUG      = _Tp*;
+  using element_type _LIBCPP_NODEBUG = _Tp;
+  using deleter_type _LIBCPP_NODEBUG = default_delete<_Tp>;
+
+private:
+  template <class _Up>
+  inline static constexpr bool __constructible_from =
+      is_convertible_v<typename _Up::pointer, pointer> &&
+      (!is_reference_v<typename _Up::deleter_type> && is_convertible_v<typename _Up::deleter_type, deleter_type>);
+
+  template <class _Up>
+  inline static constexpr bool __assignable_from =
+      is_convertible_v<typename _Up::pointer, pointer> && is_assignable_v<deleter_type&, typename _Up::deleter_type&>;
+
+public:
+  _LIBCPP_INLINE_VISIBILITY constexpr unique_ptr() = default;
+
+  _LIBCPP_INLINE_VISIBILITY constexpr unique_ptr(nullptr_t) noexcept {}
+
+  // CTAD only considers the primary template, so we can use pointer directly.
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 explicit unique_ptr(pointer __p) noexcept : __p_(__p) {}
+
+  // CTAD only considers the primary template, so we can use pointer directly.
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(pointer __p, default_delete<_Tp>) noexcept
+      : __p_(__p) {}
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr&& __other) noexcept
+      : __p_(std::exchange(__other.__p_, nullptr)) {}
+
+  template <class _Up, class _Ep, class = enable_if_t<__constructible_from<unique_ptr<_Up, _Ep>>>>
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr<_Up, _Ep>&& __other) noexcept
+      : __p_(std::exchange(__other.__p_, nullptr)) {}
+
+  template <class _Up, class _Ep>
+  _LIBCPP_INLINE_VISIBILITY unique_ptr(unique_ptr<_Up[], _Ep>&&) = delete;
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() noexcept { reset(); }
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr& operator=(unique_ptr&& __other) noexcept {
+    reset(__other.release());
+    return *this;
+  }
+
+  template <class _Up, class _Ep>
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 auto operator=(unique_ptr<_Up, _Ep>&& __other) noexcept
+      -> enable_if_t<__assignable_from<unique_ptr<_Up, _Ep>>, unique_ptr&> {
+    reset(__other.release());
+    return *this;
+  }
+
+  template <class _Up, class _Ep>
+  _LIBCPP_INLINE_VISIBILITY unique_ptr& operator=(unique_ptr<_Up[], _Ep>&&) = delete;
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr& operator=(nullptr_t) noexcept {
+    reset();
+    return *this;
+  }
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 __add_lvalue_reference(_Tp) operator*() const noexcept {
+    return *__p_;
+  }
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer operator->() const noexcept { return __p_; }
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer get() const noexcept { return __p_; }
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 deleter_type& get_deleter() noexcept { return __deleter; }
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 const deleter_type& get_deleter() const noexcept {
+    return __deleter;
+  }
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 explicit operator bool() const noexcept {
+    return __p_ != nullptr;
+  }
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer release() noexcept {
+    return std::exchange(__p_, nullptr);
+  }
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 void reset(pointer __other = nullptr) noexcept {
+    pointer __old = __p_;
+    __p_          = __other;
+    delete __old;
+  }
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 void swap(unique_ptr& other) noexcept {
+    std::swap(__p_, other.__p_);
+  }
+
+private:
+  pointer __p_ = nullptr;
+#if defined(_LIBCPP_EXACTLY_ONE_DEFAULT_DELETE_PER_TYPE)
+  inline static deleter_type __deleter;
+#else
+  deleter_type __deleter;
+#endif
+
+  template <class _Up, class _Ep>
+  friend class unique_ptr;
+};
+
+template <class _Tp>
+class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], default_delete<_Tp[]>> {
+public:
+  using pointer      = _Tp*;
+  using element_type = _Tp;
+  using deleter_type = default_delete<_Tp[]>;
+
+private:
+  template <class _Up>
+  inline static constexpr bool __pointer_compatible =
+      is_same_v<_Up, pointer> ||
+      (is_pointer_v<_Up> && is_convertible_v<remove_pointer_t<_Up> (*)[], element_type (*)[]>);
+
+  template <class _Up>
+  inline static constexpr bool __assignable =
+      is_same_v<typename _Up::pointer, typename _Up::element_type*> &&
+      is_convertible_v<typename _Up::element_type (*)[], element_type (*)[]> &&
+      is_assignable_v<deleter_type, typename _Up::deleter_type>;
+
+  template <class _Up>
+  inline static constexpr bool __constructible =
+      is_same_v<typename _Up::pointer, typename _Up::element_type*> &&
+      is_convertible_v<typename _Up::element_type (*)[], element_type (*)[]> &&
+      is_convertible_v<typename _Up::deleter_type, deleter_type>;
+
+public:
+  constexpr unique_ptr() noexcept = default;
+  constexpr unique_ptr(nullptr_t, deleter_type = {}) noexcept {}
+
+  template <class _Up, class = enable_if_t<__pointer_compatible<_Up>>>
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 explicit unique_ptr(_Up __p) noexcept : __p_(__p) {}
+
+  template <class _Up, class = enable_if_t<__pointer_compatible<_Up>>>
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(_Up __p, default_delete<_Tp[]>) noexcept : __p_(__p) {}
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr&& __other) noexcept
+      : __p_(std::exchange(__other.__p_, nullptr)) {}
+
+  template <class _Up, class _Ep, class = enable_if_t<__constructible<unique_ptr<_Up[], _Ep>>>>
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr<_Up[], _Ep>&& __other) noexcept
+      : __p_(std::exchange(__other.__ptr_.first, nullptr)) {}
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr& operator=(unique_ptr&& __other) noexcept {
+    reset(__other.release());
+    return *this;
+  }
+
+  template <class _Up, class _Ep>
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 auto operator=(unique_ptr<_Up[], _Ep>&& __other) noexcept
+      -> enable_if_t<__assignable<unique_ptr<_Up[], _Ep>>, unique_ptr&> {
+    reset(__other.release());
+    return *this;
+  }
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr& operator=(nullptr_t) noexcept {
+    reset();
+    return *this;
+  }
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 _Tp& operator[](const size_t __i) const noexcept { return __p_[__i]; }
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer get() const noexcept { return __p_; }
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 deleter_type& get_deleter() noexcept { return __deleter; }
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 const deleter_type& get_deleter() const noexcept { return __deleter; }
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 explicit operator bool() const noexcept { return __p_ != nullptr; }
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer release() noexcept { return std::exchange(__p_, nullptr); }
+
+  template <class _Up>
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 auto reset(_Up __other) noexcept -> enable_if_t<__pointer_compatible<_Up>> {
+    pointer __old = std::exchange(__p_, __other);
+    delete[] __old;
+  }
+
+  constexpr void reset(nullptr_t = nullptr) noexcept {
+    delete[] __p_;
+    __p_ = nullptr;
+  }
+
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 void swap(unique_ptr& __other) noexcept { std::swap(__p_, __other.__p_); }
+
+  // disable copy from lvalue
+  unique_ptr(const unique_ptr&)            = delete;
+  unique_ptr& operator=(const unique_ptr&) = delete;
+
+private:
+  pointer __p_ = nullptr;
+#if defined(_LIBCPP_EXACTLY_ONE_DEFAULT_DELETE_PER_TYPE)
+  inline static deleter_type __deleter;
+#else
+  deleter_type __deleter;
+#endif
+
+  template <class _Up, class _Ep>
+  friend class unique_ptr;
+};
+
 template <class _Tp, class _Dp>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23
     typename enable_if< __is_swappable<_Dp>::value, void >::type
     swap(unique_ptr<_Tp, _Dp>& __x, unique_ptr<_Tp, _Dp>& __y) _NOEXCEPT {
   __x.swap(__y);
 }
 
 template <class _T1, class _D1, class _T2, class _D2>