[libcxx-commits] [libcxx] Unconditionally lower std::string's alignment requirement from 16 to 8. (PR #68925)

[via libcxx-commits]

https://github.com/EricWF updated https://github.com/llvm/llvm-project/pull/68925

>From 5f9cb83d5c8af856be0725fcb7da578c9d566953 Mon Sep 17 00:00:00 2001
From: Eric Fiselier <eric at efcs.ca>
Date: Mon, 22 Jan 2024 16:37:40 -0600
Subject: [PATCH] Unconditionally change std::string's alignment to 8.

    Save memory by providing the allocator more freedom to allocate the most
    efficient size class by dropping the alignment requirements for std::string's
    pointer from 16 to 8. This changes the output of std::string::max_size,
    which makes it ABI breaking.

    That said, the discussion concluded that we don't care about this ABI
    break and would like this change enabled universally.

    The ABI break isn't one of layout or "class size", but rather the value of
    "max_size()" changes, which in turn changes whether std::bad_alloc or
    std::length_error is thrown for large allocations.

    This change is the child of PR #68807, which enabled the change behind an ABI
    flag.
---
 libcxx/include/__config                              |  5 -----
 libcxx/include/string                                |  7 +------
 .../string.capacity/allocation_size.pass.cpp         | 12 ++++--------
 .../basic.string/string.capacity/max_size.pass.cpp   | 10 ++--------
 .../basic.string/string.capacity/max_size.pass.cpp   |  6 ++++++
 5 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/libcxx/include/__config b/libcxx/include/__config
index 9557e8e8cf97f28..1b91734b0a5ebc3 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -174,11 +174,6 @@
 // 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
-// Save memory by providing the allocator more freedom to allocate the most
-// efficient size class by dropping the alignment requirements for std::string's
-// pointer from 16 to 8. This changes the output of std::string::max_size,
-// which makes it ABI breaking
-#    define _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
 #  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
diff --git a/libcxx/include/string b/libcxx/include/string
index 4116f350a804764..6488b5742b37612 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1931,12 +1931,7 @@ private:
     return (__s + (__a - 1)) & ~(__a - 1);
   }
   enum {
-    __alignment =
-#ifdef _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
-        8
-#else
-        16
-#endif
+    __alignment = 8
   };
   static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type __recommend(size_type __s) _NOEXCEPT {
     if (__s < __min_cap) {
diff --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
index c7df56c815a8054..1110e3d3ec568a2 100644
--- a/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
+++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
@@ -6,6 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+// XFAIL: stdlib=apple-libc++ && target={{.+}}-apple-macosx{{10.13|10.15|11.0}}
+
 // <string>
 
 // This test demonstrates the smaller allocation sizes when the alignment
@@ -17,14 +19,8 @@
 
 #include "test_macros.h"
 
-// alignment of the string heap buffer is hardcoded to either 16 or 8
-
-const std::size_t alignment =
-#ifdef _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
-    8;
-#else
-    16;
-#endif
+// alignment of the string heap buffer is hardcoded to 8
+const std::size_t alignment = 8;
 
 int main(int, char**) {
   std::string input_string;
diff --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
index a3cb79522f2e1eb..726570beb6d1ae6 100644
--- a/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
+++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
@@ -17,14 +17,8 @@
 
 #include "test_macros.h"
 
-// alignment of the string heap buffer is hardcoded to 16
-
-static const std::size_t alignment =
-#ifdef _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
-    8;
-#else
-    16;
-#endif
+// alignment of the string heap buffer is hardcoded to 8
+static const std::size_t alignment = 8;
 
 template <class = int>
 TEST_CONSTEXPR_CXX20 void full_size() {
diff --git a/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp b/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp
index 52dbde45dbb265d..32ce1c8bf617dcb 100644
--- a/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp
@@ -7,6 +7,12 @@
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: no-exceptions
+
+// After changing the alignment of the allocated pointer from 16 to 8, the exception thrown is no longer `bad_alloc`
+// but instead length_error on systems using new headers but older dylibs.
+//
+// XFAIL: stdlib=apple-libc++ && target={{.+}}-apple-macosx{{10.13|10.15|11.0}}
+
 // <string>
 
 // size_type max_size() const; // constexpr since C++20