[libcxx-commits] [libcxx] 71e4d43 - [libc++] Make sure std::allocator<void> is always trivial

[Louis Dionne via libcxx-commits]

Author: Louis Dionne
Date: 2021-06-17T16:11:50-04:00
New Revision: 71e4d434dc83b02a853712a5cb026ee2fa9ba67f

URL: https://github.com/llvm/llvm-project/commit/71e4d434dc83b02a853712a5cb026ee2fa9ba67f
DIFF: https://github.com/llvm/llvm-project/commit/71e4d434dc83b02a853712a5cb026ee2fa9ba67f.diff

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

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.

Differential Revision: https://reviews.llvm.org/D104398

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

Modified: 
    libcxx/include/__memory/allocator.h

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__memory/allocator.h b/libcxx/include/__memory/allocator.h
index 272e88f8c1a2e..cbce817d6d28d 100644
--- a/libcxx/include/__memory/allocator.h
+++ b/libcxx/include/__memory/allocator.h
@@ -52,10 +52,35 @@ class _LIBCPP_TEMPLATE_VIS allocator<const void>
 };
 #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;
@@ -65,7 +90,7 @@ class _LIBCPP_TEMPLATE_VIS allocator
     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
@@ -137,6 +162,7 @@ class _LIBCPP_TEMPLATE_VIS allocator
 
 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;
@@ -146,7 +172,7 @@ class _LIBCPP_TEMPLATE_VIS allocator<const _Tp>
     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

diff  --git a/libcxx/test/libcxx/memory/allocator_void.trivial.compile.pass.cpp b/libcxx/test/libcxx/memory/allocator_void.trivial.compile.pass.cpp
new file mode 100644
index 0000000000000..f9d67c065de85
--- /dev/null
+++ b/libcxx/test/libcxx/memory/allocator_void.trivial.compile.pass.cpp
@@ -0,0 +1,34 @@
+//===----------------------------------------------------------------------===//
+//
+// 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, "");