[libc++] Use [[no_unique_address]] in __alloc_func

[philnik]

Summary

And some code cleanup

[jloser]

FWIW, I would not mix NFC code reformatting with an unrelated change (like marking the allocator member with [[no_unique_address]]). I'd prefer to just make the two-line change in this PR, and then an NFC reformat PR as a follow-up.

[philnik]

In D119281#3305989, @jloser wrote:

FWIW, I would not mix NFC code reformatting with an unrelated change (like marking the allocator member with [[no_unique_address]]). I'd prefer to just make the two-line change in this PR, and then an NFC reformat PR as a follow-up.

I have to edit every single function anyways, but maybe it would be better.

[ldionne]

Unfortunately, we can't make this change because it breaks the ABI. We can't modify the layout of __alloc_func because existing programs could have inlined assumptions about its layout.

The layout changes notably for empty classes that are final -- those won't be elided with __compressed_pair, but they will with [[no_unique_address]].

libcxx/include/__functional/function.h

137

Does Clang really support this in earlier standards?

[philnik]

In D119281#3306015, @ldionne wrote:

Unfortunately, we can't make this change because it breaks the ABI. We can't modify the layout of __alloc_func because existing programs could have inlined assumptions about its layout.

The layout changes notably for empty classes that are final -- those won't be elided with __compressed_pair, but they will with [[no_unique_address]].

That really is unfortunate.

libcxx/include/__functional/function.h

137

Yes! https://godbolt.org/z/W9c3erW59

libcxx/include/__functional/function.h

 
 template <class _Fp, class _Ap, class _FB> class __alloc_func;
 template <class _Fp, class _FB>
 class __default_alloc_func;
 
 template <class _Fp, class _Ap, class _Rp, class... _ArgTypes>
 class __alloc_func<_Fp, _Ap, _Rp(_ArgTypes...)>
 {
-    __compressed_pair<_Fp, _Ap> __f_;
+    _Fp __func;
+    [[no_unique_address]] _Ap __alloc;
 
   public:
-    typedef _LIBCPP_NODEBUG _Fp _Target;
-    typedef _LIBCPP_NODEBUG _Ap _Alloc;
+    using _Target = _LIBCPP_NODEBUG _Fp;
+    using _Alloc = _LIBCPP_NODEBUG _Ap;
 
-    _LIBCPP_INLINE_VISIBILITY
-    const _Target& __target() const { return __f_.first(); }
+    _LIBCPP_HIDE_FROM_ABI const _Target& __target() const { return __func; }
 
     // WIN32 APIs may define __allocator, so use __get_allocator instead.
-    _LIBCPP_INLINE_VISIBILITY
-    const _Alloc& __get_allocator() const { return __f_.second(); }
-
-    _LIBCPP_INLINE_VISIBILITY
-    explicit __alloc_func(_Target&& __f)
-        : __f_(piecewise_construct, _VSTD::forward_as_tuple(_VSTD::move(__f)),
-               _VSTD::forward_as_tuple())
-    {
-    }
+    _LIBCPP_HIDE_FROM_ABI const _Alloc& __get_allocator() const { return __alloc; }
 
-    _LIBCPP_INLINE_VISIBILITY
-    explicit __alloc_func(const _Target& __f, const _Alloc& __a)
-        : __f_(piecewise_construct, _VSTD::forward_as_tuple(__f),
-               _VSTD::forward_as_tuple(__a))
-    {
-    }
+    _LIBCPP_HIDE_FROM_ABI explicit __alloc_func(_Target&& __f) : __func(std::move(__f)), __alloc() {}
+    _LIBCPP_HIDE_FROM_ABI explicit __alloc_func(const _Target& __f, const _Alloc& __a) : __func(__f), __alloc(__a) {}
 
-    _LIBCPP_INLINE_VISIBILITY
-    explicit __alloc_func(const _Target& __f, _Alloc&& __a)
-        : __f_(piecewise_construct, _VSTD::forward_as_tuple(__f),
-               _VSTD::forward_as_tuple(_VSTD::move(__a)))
-    {
-    }
+    _LIBCPP_HIDE_FROM_ABI explicit __alloc_func(const _Target& __f, _Alloc&& __a)
+        : __func(__f), __alloc(std::move(__a)) {}
 
-    _LIBCPP_INLINE_VISIBILITY
-    explicit __alloc_func(_Target&& __f, _Alloc&& __a)
-        : __f_(piecewise_construct, _VSTD::forward_as_tuple(_VSTD::move(__f)),
-               _VSTD::forward_as_tuple(_VSTD::move(__a)))
-    {
-    }
+    _LIBCPP_HIDE_FROM_ABI explicit __alloc_func(_Target&& __f, _Alloc&& __a)
+        : __func(std::move(__f)), __alloc(std::move(__a)) {}
 
-    _LIBCPP_INLINE_VISIBILITY
-    _Rp operator()(_ArgTypes&&... __arg)
-    {
-        typedef __invoke_void_return_wrapper<_Rp> _Invoker;
-        return _Invoker::__call(__f_.first(),
-                                _VSTD::forward<_ArgTypes>(__arg)...);
+    _LIBCPP_HIDE_FROM_ABI _Rp operator()(_ArgTypes&&... __arg) {
+        using _Invoker = __invoke_void_return_wrapper<_Rp>;
+        return _Invoker::__call(__func, std::forward<_ArgTypes>(__arg)...);
     }
 
-    _LIBCPP_INLINE_VISIBILITY
-    __alloc_func* __clone() const
-    {
+    _LIBCPP_HIDE_FROM_ABI __alloc_func* __clone() const {
         typedef allocator_traits<_Alloc> __alloc_traits;
-        typedef
-            typename __rebind_alloc_helper<__alloc_traits, __alloc_func>::type
-                _AA;
-        _AA __a(__f_.second());
+        typedef typename __rebind_alloc_helper<__alloc_traits, __alloc_func>::type _AA;
+        _AA __a(__alloc);
         typedef __allocator_destructor<_AA> _Dp;
         unique_ptr<__alloc_func, _Dp> __hold(__a.allocate(1), _Dp(__a, 1));
-        ::new ((void*)__hold.get()) __alloc_func(__f_.first(), _Alloc(__a));
+        ::new ((void*)__hold.get()) __alloc_func(__func, _Alloc(__a));
         return __hold.release();
     }
 
-    _LIBCPP_INLINE_VISIBILITY
-    void destroy() _NOEXCEPT { __f_.~__compressed_pair<_Target, _Alloc>(); }
+    _LIBCPP_HIDE_FROM_ABI void destroy() noexcept {
+      __func.~_Target();
+      __alloc.~_Alloc();
+    }
 
     static void __destroy_and_delete(__alloc_func* __f) {
       typedef allocator_traits<_Alloc> __alloc_traits;
-      typedef typename __rebind_alloc_helper<__alloc_traits, __alloc_func>::type
-          _FunAlloc;
+      typedef typename __rebind_alloc_helper<__alloc_traits, __alloc_func>::type _FunAlloc;
       _FunAlloc __a(__f->__get_allocator());
       __f->destroy();
       __a.deallocate(__f, 1);
     }
 };
 
 template <class _Fp, class _Rp, class... _ArgTypes>
 class __default_alloc_func<_Fp, _Rp(_ArgTypes...)> {