[libc++] Make sure std::allocator<void> is always trivial

[ldionne]

Summary

When we removed the allocator<void> specialization, the triviality of
std::allocator<void> changed because the primary template had a
non-trivial default constructor and the specialization didn't
(so std::allocator<void> went from trivial to non-trivial).

This commit fixes that oversight by giving a trivial constructor to
the primary template when instantiated on cv-void.

This was reported in https://llvm.org/PR50299.

[ldionne]

libcxx/include/__memory/allocator.h

83

@Quuxplusone I'd like you to double-check my reasoning here (see comment above). After thinking about it, I don't think this is an ABI break because of the subtlety where no two types are going to inherit from the same __non_trivial_if base class (unless they are the same type, in which case they *already* don't share the same address). So I think this is good, but an extra pair of eyes certainly won't hurt.

Also note that I would have used a trailing-requires-clause in C++20 instead of going through those base class hoops, but https://llvm.org/PR50740 prevents me from doing so.

[• Quuxplusone]

libcxx/include/__memory/allocator.h

83

Sure, LGTM. I haven't thought of any way for this to go wrong.

[ldionne]

Thanks, will ship once CI is green. I'll also cherry-pick this one to release/12.x.

libcxx/include/__memory/allocator.h

public:
     _LIBCPP_DEPRECATED_IN_CXX17 typedef const void*       pointer;
     _LIBCPP_DEPRECATED_IN_CXX17 typedef const void*       const_pointer;
     _LIBCPP_DEPRECATED_IN_CXX17 typedef const void        value_type;
 
     template <class _Up> struct _LIBCPP_DEPRECATED_IN_CXX17 rebind {typedef allocator<_Up> other;};
 };
 #endif
 
+// This class provides a non-trivial default constructor to the class that derives from it
+// if the condition is satisfied.
+//
+// The second template parameter exists to allow giving a unique type to __non_trivial_if,
+// which makes it possible to avoid breaking the ABI when making this a base class of an
+// existing class. Without that, imagine we have classes D1 and D2, both of which used to
+// have no base classes, but which now derive from __non_trivial_if. The layout of a class
+// that inherits from both D1 and D2 will change because the two __non_trivial_if base
+// classes are not allowed to share the same address.
+//
+// By making those __non_trivial_if base classes unique, we work around this problem and
+// it is safe to start deriving from __non_trivial_if in existing classes.
+template <bool _Cond, class _Unique>
+struct __non_trivial_if { };
+
+template <class _Unique>
+struct __non_trivial_if<true, _Unique> {
+    _LIBCPP_INLINE_VISIBILITY
+    _LIBCPP_CONSTEXPR __non_trivial_if() _NOEXCEPT { }
+};
+
 // allocator
+//
+// Note: For ABI compatibility between C++20 and previous standards, we make
+//       allocator<void> trivial in C++20.
 
 template <class _Tp>
 class _LIBCPP_TEMPLATE_VIS allocator
+    : private __non_trivial_if<!is_void<_Tp>::value, allocator<_Tp> >
 {
 public:
     typedef size_t      size_type;
     typedef ptrdiff_t   difference_type;
     typedef _Tp         value_type;
     typedef true_type   propagate_on_container_move_assignment;
     typedef true_type   is_always_equal;
 
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
-    allocator() _NOEXCEPT { }
+    allocator() _NOEXCEPT _LIBCPP_DEFAULT
 
     template <class _Up>
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
     allocator(const allocator<_Up>&) _NOEXCEPT { }
 
     _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
     _Tp* allocate(size_t __n) {
         if (__n > allocator_traits<allocator>::max_size(*this))
#if _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_MEMBERS)
     void destroy(pointer __p) {
         __p->~_Tp();
     }
 #endif
 };
 
 template <class _Tp>
 class _LIBCPP_TEMPLATE_VIS allocator<const _Tp>
+    : private __non_trivial_if<!is_void<_Tp>::value, allocator<const _Tp> >
 {
 public:
     typedef size_t      size_type;
     typedef ptrdiff_t   difference_type;
     typedef const _Tp   value_type;
     typedef true_type   propagate_on_container_move_assignment;
     typedef true_type   is_always_equal;
 
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
-    allocator() _NOEXCEPT { }
+    allocator() _NOEXCEPT _LIBCPP_DEFAULT
 
     template <class _Up>
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
     allocator(const allocator<_Up>&) _NOEXCEPT { }
 
     _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
     const _Tp* allocate(size_t __n) {
         if (__n > allocator_traits<allocator>::max_size(*this))

libcxx/test/libcxx/memory/allocator_void.trivial.compile.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Make sure that std::allocator<void> is trivial. This was the case before C++20
+// with the std::allocator<void> explicit specialization, and this test makes sure
+// that we maintain that property across all standards.
+//
+// This is important since triviality has implications on how the type is passed
+// as a function argument in the ABI.
+
+#include <memory>
+#include <type_traits>
+
+typedef std::allocator<void> A1;
+typedef std::allocator<void const> A2;
+struct A3 : std::allocator<void> { };
+struct A4 : std::allocator<void const> { };
+
+static_assert(std::is_trivially_default_constructible<A1>::value, "");
+static_assert(std::is_trivial<A1>::value, "");
+
+static_assert(std::is_trivially_default_constructible<A2>::value, "");
+static_assert(std::is_trivial<A2>::value, "");
+
+static_assert(std::is_trivially_default_constructible<A3>::value, "");
+static_assert(std::is_trivial<A3>::value, "");
+
+static_assert(std::is_trivially_default_constructible<A4>::value, "");
+static_assert(std::is_trivial<A4>::value, "");