[libc++] Test the size of basic_string

[ldionne]

LGTM but I'd like to add a test for alignment too, since that would be so easy.

libcxx/test/libcxx/strings/basic.string/string_abi.compile.pass.cpp

9 ↗

(On Diff #436468)

While we're at it, I think it would be trivial to add a test for the alignment. WDYT?

18 ↗

(On Diff #436468)

Space between > > here and below should help pass in C++03.

25 ↗

(On Diff #436468)

You need a dummy , ""); message in your static_assert for C++11.

[philnik]

• Address comments

libcxx/test/libcxx/strings/basic.string/string_abi.compile.pass.cpp

9 ↗

(On Diff #436468)

Sounds like a good idea.

[philnik]

• Try to fix CI

[philnik]

• Next try

[philnik]

Only update the .clang-format

[philnik]

Upload the correct diff

[philnik]

• Another try

[philnik]

• Try to find out what the correct values are on AIX

[philnik]

• A

[hubert.reinterpretcast]

libcxx/test/libcxx/strings/basic.string/sizeof.compile.pass.cpp

60

At least historically, the value is 12.

[hubert.reinterpretcast]

libcxx/test/libcxx/strings/basic.string/sizeof.compile.pass.cpp

58

[hubert.reinterpretcast]

libcxx/test/libcxx/strings/basic.string/alignof.compile.pass.cpp

100

This used to pass on AIX, but changes to use bitfields (without applying _LIBCPP_PACKED_BYTE_FOR_AIX) has broken this.

112

Same comment as above.

libcxx/test/libcxx/strings/basic.string/sizeof.compile.pass.cpp

110

This used to pass on AIX, but changes to use bitfields (without applying _LIBCPP_PACKED_BYTE_FOR_AIX) has broken this.

122

Same comment as above.

142

Same comment as above.

[hubert.reinterpretcast]

libcxx/test/libcxx/strings/basic.string/alignof.compile.pass.cpp

100

However, using the above also causes other members to be less aligned if padding is not explicitly added.

[hubert.reinterpretcast]

libcxx/test/libcxx/strings/basic.string/alignof.compile.pass.cpp

100

Or rather it causes the string type to be less aligned.

[hubert.reinterpretcast]

libcxx/test/libcxx/strings/basic.string/alignof.compile.pass.cpp

100

--- string_20220520     2022-05-20 11:48:54.326820970 -0400
+++ string      2022-06-16 19:30:22.238194020 -0400
@@ -724,3 +724,4 @@

-    struct __long
+_LIBCPP_PACKED_BYTE_FOR_AIX
+    struct __long_impl
     {
@@ -729,2 +730,6 @@
         size_type __size_;
+        char
+            __padding_[sizeof(pointer) *
+                           ((2 * sizeof(size_type) - 1) / sizeof(pointer) + 1) -
+                       2 * sizeof(size_type)];
         pointer   __data_;
@@ -731,2 +736,4 @@
     };
+_LIBCPP_PACKED_BYTE_FOR_AIX_END
+    struct alignas(size_type) alignas(pointer) __long : __long_impl {};

@@ -735,3 +742,4 @@

-    struct __short
+_LIBCPP_PACKED_BYTE_FOR_AIX
+    struct __short_impl
     {
@@ -742,2 +750,4 @@
     };
+_LIBCPP_PACKED_BYTE_FOR_AIX_END
+    struct alignas(value_type) __short : __short_impl {};

[philnik]

Change back to the old diff. D128285 fixed the basic_string layout, so the test should pass now.

[philnik]

@hubert.reinterpretcast Do you know what the correct values for AIX are? If that's the case, could you tell me? Or how about I land this with XFAIL: AIX any you fix it in a follow-up patch?

[hubert.reinterpretcast]

In D127672#3650406, @philnik wrote:

@hubert.reinterpretcast Do you know what the correct values for AIX are? If that's the case, could you tell me? Or how about I land this with XFAIL: AIX any you fix it in a follow-up patch?

As far as I can tell, the values are correct. I will need to see if the failures show up when using the LIT harness.

[hubert.reinterpretcast]

In D127672#3650566, @hubert.reinterpretcast wrote:

As far as I can tell, the values are correct. I will need to see if the failures show up when using the LIT harness.

These pass for me with the LIT harness. Perhaps the precommit CI does make use of the "Parents" metadata?
The metadata shows that your patch is a commit on top of rGaf41955a49725c8ddc69d94b1874bbbbd608729e.
Xing's fix in rG60f7bdfd0317cb38eda54637cdab9baed8ae2f57 occurs after that.

My guess is that the CI will pass if you rebase.

[philnik]

Rebased

[philnik]

In D127672#3650694, @hubert.reinterpretcast wrote:

In D127672#3650566, @hubert.reinterpretcast wrote:

As far as I can tell, the values are correct. I will need to see if the failures show up when using the LIT harness.

These pass for me with the LIT harness. Perhaps the precommit CI does make use of the "Parents" metadata?
The metadata shows that your patch is a commit on top of rGaf41955a49725c8ddc69d94b1874bbbbd608729e.
Xing's fix in rG60f7bdfd0317cb38eda54637cdab9baed8ae2f57 occurs after that.

My guess is that the CI will pass if you rebase.

Thanks! I thought I had already rebased, but apparently not.

libcxx/test/libcxx/strings/basic.string/alignof.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
+//
+//===----------------------------------------------------------------------===//
+
+// Ensure that we never change the size or alignment of `basic_string`
+
+// UNSUPPORTED: c++03
+
+#include <string>
+
+#include "test_macros.h"
+#include "min_allocator.h"
+#include "test_allocator.h"
+
+template <class T>
+class small_pointer {
+  uint16_t offset;
+};
+
+template <class T>
+class small_iter_allocator {
+public:
+  using value_type = T;
+  using pointer = small_pointer<T>;
+  using size_type = int16_t;
+  using difference_type = int16_t;
+
+  small_iter_allocator() TEST_NOEXCEPT {}
+
+  template <class U>
+  small_iter_allocator(small_iter_allocator<U>) TEST_NOEXCEPT {}
+
+  T* allocate(std::size_t n);
+  void deallocate(T* p, std::size_t);
+
+  friend bool operator==(small_iter_allocator, small_iter_allocator) { return true; }
+  friend bool operator!=(small_iter_allocator, small_iter_allocator) { return false; }
+};
+
+template <class CharT>
+using min_string = std::basic_string<CharT, std::char_traits<CharT>, min_allocator<CharT>>;
+
+template <class CharT>
+using test_string = std::basic_string<CharT, std::char_traits<CharT>, test_allocator<CharT>>;
+
+template <class CharT>
+using small_string = std::basic_string<CharT, std::char_traits<CharT>, small_iter_allocator<CharT>>;
+
+#if __SIZE_WIDTH__ == 64
+
+static_assert(alignof(std::string) == 8, "");
+static_assert(alignof(min_string<char>) == 8, "");
+static_assert(alignof(test_string<char>) == 8, "");
+static_assert(alignof(small_string<char>) == 2, "");
+
+#  ifndef TEST_HAS_NO_WIDE_CHARACTERS
+#    if __WCHAR_WIDTH__ == 32
+static_assert(alignof(std::wstring) == 8, "");
+static_assert(alignof(min_string<wchar_t>) == 8, "");
+static_assert(alignof(test_string<wchar_t>) == 8, "");
+static_assert(alignof(small_string<wchar_t>) == 4, "");
+#    elif __WCHAR_WIDTH__ == 16
+static_assert(alignof(std::wstring) == 8, "");
+static_assert(alignof(min_string<wchar_t>) == 8, "");
+static_assert(alignof(test_string<wchar_t>) == 8, "");
+static_assert(alignof(small_string<wchar_t>) == 2, "");
+#    else
+#      error "Unexpected wchar_t width"
+#    endif
+#  endif
+
+#  ifndef TEST_HAS_NO_CHAR8_T
+static_assert(alignof(std::u8string) == 8, "");
+static_assert(alignof(min_string<char8_t>) == 8, "");
+static_assert(alignof(test_string<char8_t>) == 8, "");
+static_assert(alignof(small_string<char8_t>) == 2, "");
+#  endif
+
+#  ifndef TEST_HAS_NO_UNICODE_CHARS
+static_assert(alignof(std::u16string) == 8, "");
+static_assert(alignof(std::u32string) == 8, "");
+static_assert(alignof(min_string<char16_t>) == 8, "");
+static_assert(alignof(min_string<char32_t>) == 8, "");
+static_assert(alignof(test_string<char16_t>) == 8, "");
+static_assert(alignof(test_string<char32_t>) == 8, "");
+static_assert(alignof(small_string<char16_t>) == 2, "");
+static_assert(alignof(small_string<char32_t>) == 4, "");
+#  endif
+
+#elif __SIZE_WIDTH__ == 32
+
+static_assert(alignof(std::string) == 4, "");
+static_assert(alignof(min_string<char>) == 4, "");
+static_assert(alignof(test_string<char>) == 4, "");
+static_assert(alignof(small_string<char>) == 2, "");
+
+#  ifndef TEST_HAS_NO_WIDE_CHARACTERS
+#    if __WCHAR_WIDTH__ == 32
+static_assert(alignof(std::wstring) == 4, "");
+static_assert(alignof(min_string<wchar_t>) == 4, "");
+static_assert(alignof(test_string<wchar_t>) == 4, "");
+static_assert(alignof(small_string<wchar_t>) == 4, "");
+#    elif __WCHAR_WIDTH__ == 16
+static_assert(alignof(std::wstring) == 4, "");
+static_assert(alignof(min_string<wchar_t>) == 4, "");
+static_assert(alignof(test_string<wchar_t>) == 4, "");
+static_assert(alignof(small_string<wchar_t>) == 2, "");
+#    else
+#      error "Unexpected wchar_t width"
+#    endif
+#  endif
+
+#  ifndef TEST_HAS_NO_CHAR8_T
+static_assert(alignof(std::u8string) == 4, "");
+static_assert(alignof(min_string<char8_t>) == 4, "");
+static_assert(alignof(test_string<char8_t>) == 4, "");
+static_assert(alignof(small_string<char8_t>) == 2, "");
+#  endif
+
+#  ifndef TEST_HAS_NO_UNICODE_CHARS
+static_assert(alignof(std::u16string) == 4, "");
+static_assert(alignof(std::u32string) == 4, "");
+static_assert(alignof(min_string<char16_t>) == 4, "");
+static_assert(alignof(min_string<char32_t>) == 4, "");
+static_assert(alignof(test_string<char16_t>) == 4, "");
+static_assert(alignof(test_string<char32_t>) == 4, "");
+static_assert(alignof(small_string<char32_t>) == 4, "");
+#  endif
+
+#else
+#  error "size_t has an unexpected size"
+#endif

libcxx/test/libcxx/strings/basic.string/sizeof.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
+//
+//===----------------------------------------------------------------------===//
+
+// Ensure that we never change the size or alignment of `basic_string`
+
+#include <string>
+
+#include "test_macros.h"
+#include "min_allocator.h"
+#include "test_allocator.h"
+
+template <class T>
+class small_pointer {
+  uint16_t offset;
+};
+
+template <class T>
+class small_iter_allocator {
+public:
+  using value_type = T;
+  using pointer = small_pointer<T>;
+  using size_type = int16_t;
+  using difference_type = int16_t;
+
+  small_iter_allocator() TEST_NOEXCEPT {}
+
+  template <class U>
+  small_iter_allocator(small_iter_allocator<U>) TEST_NOEXCEPT {}
+
+  T* allocate(std::size_t n);
+  void deallocate(T* p, std::size_t);
+
+  friend bool operator==(small_iter_allocator, small_iter_allocator) { return true; }
+  friend bool operator!=(small_iter_allocator, small_iter_allocator) { return false; }
+};
+
+template <class CharT>
+using min_string = std::basic_string<CharT, std::char_traits<CharT>, min_allocator<CharT> >;
+
+template <class CharT>
+using test_string = std::basic_string<CharT, std::char_traits<CharT>, test_allocator<CharT> >;
+
+template <class CharT>
+using small_string = std::basic_string<CharT, std::char_traits<CharT>, small_iter_allocator<CharT> >;
+
+#if __SIZE_WIDTH__ == 64
+
+static_assert(sizeof(std::string) == 24, "");
+static_assert(sizeof(min_string<char>) == 24, "");
+static_assert(sizeof(test_string<char>) == 32, "");
+static_assert(sizeof(small_string<char>) == 6, "");
+
+#  ifndef TEST_HAS_NO_WIDE_CHARACTERS
+#    if __WCHAR_WIDTH__ == 32
+static_assert(sizeof(std::wstring) == 24, "");
+static_assert(sizeof(min_string<wchar_t>) == 24, "");
+static_assert(sizeof(test_string<wchar_t>) == 32, "");
+static_assert(sizeof(small_string<wchar_t>) == 12, "");
+#    elif __WCHAR_WIDTH__ == 16
+static_assert(sizeof(std::wstring) == 24, "");
+static_assert(sizeof(min_string<wchar_t>) == 24, "");
+static_assert(sizeof(test_string<wchar_t>) == 32, "");
+static_assert(sizeof(small_string<wchar_t>) == 6, "");
+#    else
+#      error "Unexpected wchar_t width"
+#    endif
+#  endif
+
+#  ifndef TEST_HAS_NO_CHAR8_T
+static_assert(sizeof(std::u8string) == 24, "");
+static_assert(sizeof(min_string<char8_t>) == 24, "");
+static_assert(sizeof(test_string<char8_t>) == 32, "");
+static_assert(sizeof(small_string<char8_t>) == 6, "");
+#  endif
+
+#  ifndef TEST_HAS_NO_UNICODE_CHARS
+static_assert(sizeof(std::u16string) == 24, "");
+static_assert(sizeof(std::u32string) == 24, "");
+static_assert(sizeof(min_string<char16_t>) == 24, "");
+static_assert(sizeof(min_string<char32_t>) == 24, "");
+static_assert(sizeof(test_string<char16_t>) == 32, "");
+static_assert(sizeof(test_string<char32_t>) == 32, "");
+static_assert(sizeof(small_string<char16_t>) == 6, "");
+static_assert(sizeof(small_string<char32_t>) == 12, "");
+#  endif
+
+#elif __SIZE_WIDTH__ == 32
+
+static_assert(sizeof(std::string) == 12, "");
+static_assert(sizeof(min_string<char>) == 12, "");
+static_assert(sizeof(test_string<char>) == 24, "");
+static_assert(sizeof(small_string<char>) == 6, "");
+
+#  ifndef TEST_HAS_NO_WIDE_CHARACTERS
+#    if __WCHAR_WIDTH__ == 32
+static_assert(sizeof(std::wstring) == 12, "");
+static_assert(sizeof(min_string<wchar_t>) == 12, "");
+static_assert(sizeof(test_string<wchar_t>) == 24, "");
+static_assert(sizeof(small_string<wchar_t>) == 12, "");
+#    elif __WCHAR_WIDTH__ == 16
+static_assert(sizeof(std::wstring) == 12, "");
+static_assert(sizeof(min_string<wchar_t>) == 12, "");
+static_assert(sizeof(test_string<wchar_t>) == 24, "");
+static_assert(sizeof(small_string<wchar_t>) == 6, "");
+#    else
+#      error "Unexpected wchar_t width"
+#    endif
+#  endif
+
+#  ifndef TEST_HAS_NO_CHAR8_T
+static_assert(sizeof(std::u8string) == 12, "");
+static_assert(sizeof(min_string<char8_t>) == 12, "");
+static_assert(sizeof(test_string<char8_t>) == 24, "");
+static_assert(sizeof(small_string<char>) == 6, "");
+#  endif
+
+#  ifndef TEST_HAS_NO_UNICODE_CHARS
+static_assert(sizeof(std::u16string) == 12, "");
+static_assert(sizeof(std::u32string) == 12, "");
+static_assert(sizeof(min_string<char16_t>) == 12, "");
+static_assert(sizeof(min_string<char32_t>) == 12, "");
+static_assert(sizeof(test_string<char16_t>) == 24, "");
+static_assert(sizeof(test_string<char32_t>) == 24, "");
+static_assert(sizeof(small_string<char16_t>) == 6, "");
+static_assert(sizeof(small_string<char32_t>) == 12, "");
+#  endif
+
+#else
+#  error "size_t has an unexpected size"
+#endif