Skip to content

Conversation

@ldionne
Copy link
Member

@ldionne ldionne commented Nov 27, 2024

This patch refactors the tests around aligned allocation and sized deallocation to avoid relying on passing the -fsized-deallocation or -faligned-allocation flags by default. Since both of these features are enabled by default in >= C++14 mode, it now makes sense to make that assumption in the test suite.

A notable exception is MinGW and some older compilers, where sized deallocation is still not enabled by default. We treat that as a "bug" in the test suite and we work around it by explicitly adding -fsized-deallocation, but only under those configurations.

@ldionne ldionne requested a review from a team as a code owner November 27, 2024 19:30
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 27, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

This patch refactors the tests around aligned allocation and sized deallocation to avoid relying on passing the -fsized-deallocation or -faligned-allocation flags by default. Since both of these features are enabled by default in >= C++14 mode, it now makes sense to make that assumption in the test suite.

A notable exception is MinGW and some older compilers, where sized deallocation is still not enabled by default. We treat that as a "bug" in the test suite and we work around it by explicitly adding -fsized-deallocation, but only under those configurations.


Patch is 26.19 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/117915.diff

14 Files Affected:

  • (modified) libcxx/test/benchmarks/allocation.bench.cpp (+8-2)
  • (removed) libcxx/test/libcxx/language.support/support.dynamic/new_faligned_allocation.pass.cpp (-108)
  • (modified) libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp (+7-2)
  • (modified) libcxx/test/std/language.support/support.dynamic/align_val_t.pass.cpp (+36-12)
  • (modified) libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.pass.cpp (+5-6)
  • (modified) libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.pass.cpp (+5-6)
  • (renamed) libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp (+19-6)
  • (removed) libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp (-78)
  • (modified) libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align.pass.cpp (+5-6)
  • (modified) libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp (+5-6)
  • (renamed) libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete.pass.cpp (+22-7)
  • (removed) libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete_fsizeddeallocation.pass.cpp (-71)
  • (modified) libcxx/test/std/language.support/support.dynamic/new.delete/types.h (+31)
  • (modified) libcxx/utils/libcxx/test/features.py (-8)
diff --git a/libcxx/test/benchmarks/allocation.bench.cpp b/libcxx/test/benchmarks/allocation.bench.cpp
index 66a9b88793412b..4e69b5464798a9 100644
--- a/libcxx/test/benchmarks/allocation.bench.cpp
+++ b/libcxx/test/benchmarks/allocation.bench.cpp
@@ -6,8 +6,14 @@
 //
 //===----------------------------------------------------------------------===//
 
-// REQUIRES: -fsized-deallocation
-// ADDITIONAL_COMPILE_FLAGS: -fsized-deallocation
+// UNSUPPORTED: c++03, c++11
+
+// These compiler versions and platforms don't enable sized deallocation by default.
+// ADDITIONAL_COMPILE_FLAGS(clang-17): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(clang-18): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(apple-clang-16): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(target=x86_64-w64-windows-gnu): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(target=i686-w64-windows-gnu): -fsized-deallocation
 
 #include "benchmark/benchmark.h"
 
diff --git a/libcxx/test/libcxx/language.support/support.dynamic/new_faligned_allocation.pass.cpp b/libcxx/test/libcxx/language.support/support.dynamic/new_faligned_allocation.pass.cpp
deleted file mode 100644
index 87f4783e12973e..00000000000000
--- a/libcxx/test/libcxx/language.support/support.dynamic/new_faligned_allocation.pass.cpp
+++ /dev/null
@@ -1,108 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-// test libc++'s implementation of align_val_t, and the relevant new/delete
-// overloads in all dialects when -faligned-allocation is present.
-
-// Libc++ when built for z/OS doesn't contain the aligned allocation functions,
-// nor does the dynamic library shipped with z/OS.
-// XFAIL: target={{.+}}-zos{{.*}}
-
-// REQUIRES: -faligned-allocation
-// ADDITIONAL_COMPILE_FLAGS: -faligned-allocation
-
-#include <cassert>
-#include <new>
-#include <string>
-#include <type_traits>
-#include <typeinfo>
-
-#include "test_macros.h"
-
-static void test_allocations(std::size_t size, size_t alignment) {
-  {
-    void* ptr = ::operator new(size, std::align_val_t(alignment));
-    assert(ptr);
-    assert(reinterpret_cast<std::uintptr_t>(ptr) % alignment == 0);
-    ::operator delete(ptr, std::align_val_t(alignment));
-  }
-  {
-    void* ptr = ::operator new(size, std::align_val_t(alignment), std::nothrow);
-    assert(ptr);
-    assert(reinterpret_cast<std::uintptr_t>(ptr) % alignment == 0);
-    ::operator delete(ptr, std::align_val_t(alignment), std::nothrow);
-  }
-  {
-    void* ptr = ::operator new[](size, std::align_val_t(alignment));
-    assert(ptr);
-    assert(reinterpret_cast<std::uintptr_t>(ptr) % alignment == 0);
-    ::operator delete[](ptr, std::align_val_t(alignment));
-  }
-  {
-    void* ptr = ::operator new[](size, std::align_val_t(alignment), std::nothrow);
-    assert(ptr);
-    assert(reinterpret_cast<std::uintptr_t>(ptr) % alignment == 0);
-    ::operator delete[](ptr, std::align_val_t(alignment), std::nothrow);
-  }
-}
-
-int main(int, char**) {
-  {
-    static_assert(std::is_enum<std::align_val_t>::value, "");
-    typedef std::underlying_type<std::align_val_t>::type UT;
-    static_assert((std::is_same<UT, std::size_t>::value), "");
-  }
-  {
-    static_assert((!std::is_constructible<std::align_val_t, std::size_t>::value), "");
-#if TEST_STD_VER >= 11
-    static_assert(!std::is_constructible<std::size_t, std::align_val_t>::value, "");
-#else
-    static_assert((std::is_constructible<std::size_t, std::align_val_t>::value), "");
-#endif
-  }
-  {
-    std::align_val_t a = std::align_val_t(0);
-    std::align_val_t b = std::align_val_t(32);
-    assert(a != b);
-    assert(a == std::align_val_t(0));
-    assert(b == std::align_val_t(32));
-  }
-  // First, check the basic case, a large allocation with alignment==size.
-  test_allocations(64, 64);
-  // Size being a multiple of alignment also needs to be supported.
-  test_allocations(64, 32);
-  // When aligned allocation is implemented using aligned_alloc,
-  // that function requires a minimum alignment of sizeof(void*).
-  // Check that we can also create overaligned allocations with
-  // an alignment argument less than sizeof(void*).
-  test_allocations(2, 2);
-  // When implemented using the C11 aligned_alloc() function,
-  // that requires that size be a multiple of alignment.
-  // However, the C++ operator new has no such requirements.
-  // Check that we can create an overaligned allocation that does
-  // adhere to not have this constraint.
-  test_allocations(1, 128);
-  // Finally, test size > alignment, but with size not being
-  // a multiple of alignment.
-  test_allocations(65, 32);
-#ifndef TEST_HAS_NO_RTTI
-  {
-    // Check that libc++ doesn't define align_val_t in a versioning namespace.
-    // And that it mangles the same in C++03 through C++17
-#ifdef _MSC_VER
-    // MSVC uses a different C++ ABI with a different name mangling scheme.
-    // The type id name doesn't seem to contain the mangled form at all.
-    assert(typeid(std::align_val_t).name() == std::string("enum std::align_val_t"));
-#else
-    assert(typeid(std::align_val_t).name() == std::string("St11align_val_t"));
-#endif
-  }
-#endif
-
-  return 0;
-}
diff --git a/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp b/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
index 772198304b415a..9ff148251a05f6 100644
--- a/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
+++ b/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
@@ -8,8 +8,13 @@
 //
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
-// REQUIRES: -fsized-deallocation
-// ADDITIONAL_COMPILE_FLAGS: -fsized-deallocation
+
+// These compiler versions and platforms don't enable sized deallocation by default.
+// ADDITIONAL_COMPILE_FLAGS(clang-17): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(clang-18): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(apple-clang-16): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(target=x86_64-w64-windows-gnu): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(target=i686-w64-windows-gnu): -fsized-deallocation
 
 // This test will fail with ASan if the implementation passes different sizes
 // to corresponding allocation and deallocation functions.
diff --git a/libcxx/test/std/language.support/support.dynamic/align_val_t.pass.cpp b/libcxx/test/std/language.support/support.dynamic/align_val_t.pass.cpp
index 28c72f0be7aed8..b898b37ebc7e9d 100644
--- a/libcxx/test/std/language.support/support.dynamic/align_val_t.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/align_val_t.pass.cpp
@@ -15,26 +15,50 @@
 // XFAIL: target={{.+}}-zos{{.*}}
 
 #include <new>
+#include <cassert>
+#include <cstddef>
+#include <string>
 #include <type_traits>
+#include <typeinfo>
 
 #include "test_macros.h"
 
-int main(int, char**) {
+constexpr bool test() {
+  static_assert(std::is_enum<std::align_val_t>::value, "");
+  static_assert(std::is_same<std::underlying_type<std::align_val_t>::type, std::size_t>::value, "");
+  static_assert(!std::is_constructible<std::align_val_t, std::size_t>::value, "");
+  static_assert(!std::is_constructible<std::size_t, std::align_val_t>::value, "");
+
   {
-    static_assert(std::is_enum<std::align_val_t>::value, "");
-    static_assert(std::is_same<std::underlying_type<std::align_val_t>::type, std::size_t>::value, "");
-    static_assert(!std::is_constructible<std::align_val_t, std::size_t>::value, "");
-    static_assert(!std::is_constructible<std::size_t, std::align_val_t>::value, "");
+    auto a = std::align_val_t(0);
+    auto b = std::align_val_t(32);
+    auto c = std::align_val_t(-1);
+    assert(a != b);
+    assert(a == std::align_val_t(0));
+    assert(b == std::align_val_t(32));
+    assert(static_cast<std::size_t>(c) == static_cast<std::size_t>(-1));
   }
+
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test(), "");
+
+#if defined(_LIBCPP_VERSION) && !defined(TEST_HAS_NO_RTTI)
   {
-    constexpr auto a = std::align_val_t(0);
-    constexpr auto b = std::align_val_t(32);
-    constexpr auto c = std::align_val_t(-1);
-    static_assert(a != b, "");
-    static_assert(a == std::align_val_t(0), "");
-    static_assert(b == std::align_val_t(32), "");
-    static_assert(static_cast<std::size_t>(c) == (std::size_t)-1, "");
+    // Check that libc++ doesn't define align_val_t in a versioning namespace.
+    // And that it mangles the same in C++03 through C++17
+#  ifdef _MSC_VER
+    // MSVC uses a different C++ ABI with a different name mangling scheme.
+    // The type id name doesn't seem to contain the mangled form at all.
+    assert(typeid(std::align_val_t).name() == std::string("enum std::align_val_t"));
+#  else
+    assert(typeid(std::align_val_t).name() == std::string("St11align_val_t"));
+#  endif
   }
+#endif
 
   return 0;
 }
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.pass.cpp
index dd8090aca5b285..c9b59ecaff396d 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.pass.cpp
@@ -34,13 +34,12 @@ void my_new_handler() {
 }
 
 int main(int, char**) {
-    // Test that we can call the function directly
-    {
-        void* x = operator new[](10, static_cast<std::align_val_t>(64));
+    test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
+        void* x = operator new[](size, static_cast<std::align_val_t>(alignment));
         assert(x != nullptr);
-        assert(reinterpret_cast<std::uintptr_t>(x) % 64 == 0);
-        operator delete[](x, static_cast<std::align_val_t>(64));
-    }
+        assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
+        operator delete[](x, static_cast<std::align_val_t>(alignment));
+    });
 
     // Test that the new handler is called if allocation fails
     {
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.pass.cpp
index 6ae8ceaf534e48..1bdae8d597f00f 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.pass.cpp
@@ -34,13 +34,12 @@ void my_new_handler() {
 }
 
 int main(int, char**) {
-    // Test that we can call the function directly
-    {
-        void* x = operator new[](10, static_cast<std::align_val_t>(64), std::nothrow);
+    test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
+        void* x = operator new[](size, static_cast<std::align_val_t>(alignment), std::nothrow);
         assert(x != nullptr);
-        assert(reinterpret_cast<std::uintptr_t>(x) % 64 == 0);
-        operator delete[](x, static_cast<std::align_val_t>(64), std::nothrow);
-    }
+        assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
+        operator delete[](x, static_cast<std::align_val_t>(alignment), std::nothrow);
+    });
 
     // Test that the new handler is called and we return nullptr if allocation fails
     {
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array_fsizeddeallocation.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp
similarity index 68%
rename from libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array_fsizeddeallocation.pass.cpp
rename to libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp
index 6077318278a1ba..a7991c8659f8d5 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array_fsizeddeallocation.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp
@@ -6,15 +6,28 @@
 //
 //===----------------------------------------------------------------------===//
 
-// test sized operator delete[] replacement.
+// Test sized operator delete[] replacement.
 
-// Note that sized delete operator definitions below are simply ignored
-// when sized deallocation is not supported, e.g., prior to C++14.
+// UNSUPPORTED: c++03, c++11
+
+// These compiler versions and platforms don't enable sized deallocation by default.
+// ADDITIONAL_COMPILE_FLAGS(clang-17): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(clang-18): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(apple-clang-16): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(target=x86_64-w64-windows-gnu): -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS(target=i686-w64-windows-gnu): -fsized-deallocation
+
+// Android clang-r536225 identifies as clang-19.0 but it predates the real
+// LLVM 19.0.0, so it also leaves sized deallocation off by default.
+// UNSUPPORTED: android && clang-19.0
 
 // UNSUPPORTED: sanitizer-new-delete
 
-// REQUIRES: -fsized-deallocation
-// ADDITIONAL_COMPILE_FLAGS: -fsized-deallocation
+// Sized deallocation was introduced in LLVM 11
+// XFAIL: using-built-library-before-llvm-11
+
+// AIX, and z/OS default to -fno-sized-deallocation.
+// XFAIL: target={{.+}}-aix{{.*}}, target={{.+}}-zos{{.*}}
 
 #if !defined(__cpp_sized_deallocation)
 # error __cpp_sized_deallocation should be defined
@@ -76,5 +89,5 @@ int main(int, char**)
     assert(0 == unsized_delete_nothrow_called);
     assert(1 == sized_delete_called);
 
-  return 0;
+    return 0;
 }
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp
deleted file mode 100644
index 52c4e3e0f69b69..00000000000000
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp
+++ /dev/null
@@ -1,78 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-// test sized operator delete[] replacement.
-
-// These compiler versions don't enable sized deallocation by default.
-// UNSUPPORTED: clang-17, clang-18
-
-// Android clang-r536225 identifies as clang-19.0 but it predates the real
-// LLVM 19.0.0, so it also leaves sized deallocation off by default.
-// UNSUPPORTED: android && clang-19.0
-
-// UNSUPPORTED: sanitizer-new-delete, c++03, c++11
-// XFAIL: apple-clang
-// XFAIL: using-built-library-before-llvm-11
-
-// AIX, z/OS, and MinGW default to -fno-sized-deallocation.
-// XFAIL: target={{.+}}-aix{{.*}}, target={{.+}}-zos{{.*}}, target={{.+}}-windows-gnu
-
-#include <new>
-#include <cstddef>
-#include <cstdlib>
-#include <cassert>
-
-#include "test_macros.h"
-
-int unsized_delete_called = 0;
-int unsized_delete_nothrow_called = 0;
-int sized_delete_called = 0;
-
-void operator delete[](void* p) TEST_NOEXCEPT
-{
-    ++unsized_delete_called;
-    std::free(p);
-}
-
-void operator delete[](void* p, const std::nothrow_t&) TEST_NOEXCEPT
-{
-    ++unsized_delete_nothrow_called;
-    std::free(p);
-}
-
-void operator delete[](void* p, std::size_t) TEST_NOEXCEPT
-{
-    ++sized_delete_called;
-    std::free(p);
-}
-
-// NOTE: Use a class with a non-trivial destructor as the test type in order
-// to ensure the correct overload is called.
-// C++14 5.3.5 [expr.delete]p10
-// - If the type is complete and if, for the second alternative (delete array)
-//   only, the operand is a pointer to a class type with a non-trivial
-//   destructor or a (possibly multi-dimensional) array thereof, the function
-//   with two parameters is selected.
-// - Otherwise, it is unspecified which of the two deallocation functions is
-//   selected.
-struct A { ~A() {} };
-
-int main(int, char**)
-{
-    A* x = new A[3];
-    assert(0 == unsized_delete_called);
-    assert(0 == unsized_delete_nothrow_called);
-    assert(0 == sized_delete_called);
-
-    delete [] x;
-    assert(0 == unsized_delete_called);
-    assert(0 == unsized_delete_nothrow_called);
-    assert(1 == sized_delete_called);
-
-  return 0;
-}
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align.pass.cpp
index dbb10a76ad9e9b..5d321f08282b21 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align.pass.cpp
@@ -34,13 +34,12 @@ void my_new_handler() {
 }
 
 int main(int, char**) {
-    // Test that we can call the function directly
-    {
-        void* x = operator new(10, static_cast<std::align_val_t>(64));
+    test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
+        void* x = operator new(size, static_cast<std::align_val_t>(alignment));
         assert(x != nullptr);
-        assert(reinterpret_cast<std::uintptr_t>(x) % 64 == 0);
-        operator delete(x, static_cast<std::align_val_t>(64));
-    }
+        assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
+        operator delete(x, static_cast<std::align_val_t>(alignment));
+    });
 
     // Test that the new handler is called if allocation fails
     {
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp
index b9d8ea2f4e4947..3e5ba574110fda 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp
@@ -34,13 +34,12 @@ void my_new_handler() {
 }
 
 int main(int, char**) {
-    // Test that we can call the function directly
-    {
-        void* x = operator new(10, static_cast<std::align_val_t>(64), std::nothrow);
+    test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
+        void* x = operator new(size, static_cast<std::align_val_t>(alignment), std::nothrow);
         assert(x != nullptr);
-        assert(reinterpret_cast<std::uintptr_t>(x) % 64 == 0);
-        operator delete(x, static_cast<std::align_val_t>(64), std::nothrow);
-    }
+        assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
+        operator delete(x, static_cast<std::align_val_t>(alignment), std::nothrow);
+    });
 
     // Test that the new handler is called and we return nullptr if allocation fails
     {
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete14.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete.pass.cpp
similarity index 63%
rename from libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete14.pass.cpp
rename to libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete.pass.cpp
index e00339761ec24c..85d014c07240d7 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete14.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete.pass.cpp
@@ -8,19 +8,34 @@
 
 // Test sized operator delete replacement.
 
-// These compiler versions do not enable sized deallocation by default.
-// UNSUPPORTED: clang-17, clang-18
+// UNSUPPORTED: c++03, c++11
+
+// These comp...
[truncated]

@github-actions
Copy link

github-actions bot commented Nov 27, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 02eaeec1ac61b3933bf10a40abdeff4e2958a4a5 2ac0fff67a590e17dde5a28cc4365245518a5bc0 --extensions h,cpp -- libcxx/test/benchmarks/allocation.bench.cpp libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp libcxx/test/std/language.support/support.dynamic/align_val_t.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/types.h libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete.pass.cpp
View the diff from clang-format here.
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.pass.cpp
index c9b59ecaff..8045fca37b 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.pass.cpp
@@ -34,15 +34,15 @@ void my_new_handler() {
 }
 
 int main(int, char**) {
-    test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
-        void* x = operator new[](size, static_cast<std::align_val_t>(alignment));
-        assert(x != nullptr);
-        assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
-        operator delete[](x, static_cast<std::align_val_t>(alignment));
-    });
+  test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
+    void* x = operator new[](size, static_cast<std::align_val_t>(alignment));
+    assert(x != nullptr);
+    assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
+    operator delete[](x, static_cast<std::align_val_t>(alignment));
+  });
 
-    // Test that the new handler is called if allocation fails
-    {
+  // Test that the new handler is called if allocation fails
+  {
 #ifndef TEST_HAS_NO_EXCEPTIONS
         std::set_new_handler(my_new_handler);
         try {
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.pass.cpp
index 1bdae8d597..7c410a8a55 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.pass.cpp
@@ -34,20 +34,19 @@ void my_new_handler() {
 }
 
 int main(int, char**) {
-    test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
-        void* x = operator new[](size, static_cast<std::align_val_t>(alignment), std::nothrow);
-        assert(x != nullptr);
-        assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
-        operator delete[](x, static_cast<std::align_val_t>(alignment), std::nothrow);
-    });
+  test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
+    void* x = operator new[](size, static_cast<std::align_val_t>(alignment), std::nothrow);
+    assert(x != nullptr);
+    assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
+    operator delete[](x, static_cast<std::align_val_t>(alignment), std::nothrow);
+  });
 
-    // Test that the new handler is called and we return nullptr if allocation fails
-    {
-        std::set_new_handler(my_new_handler);
-        void* x = operator new[](std::numeric_limits<std::size_t>::max(),
-                                 static_cast<std::align_val_t>(32), std::nothrow);
-        assert(new_handler_called == 1);
-        assert(x == nullptr);
+  // Test that the new handler is called and we return nullptr if allocation fails
+  {
+    std::set_new_handler(my_new_handler);
+    void* x = operator new[](std::numeric_limits<std::size_t>::max(), static_cast<std::align_val_t>(32), std::nothrow);
+    assert(new_handler_called == 1);
+    assert(x == nullptr);
     }
 
     // Test that a new expression constructs the right object
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp
index f0ad2c0e67..548b82e565 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp
@@ -31,11 +31,11 @@
 // XFAIL: target={{.+}}-aix{{.*}}, target={{.+}}-zos{{.*}}
 
 #if !defined(__cpp_sized_deallocation)
-# error __cpp_sized_deallocation should be defined
+#  error __cpp_sized_deallocation should be defined
 #endif
 
 #if !(__cpp_sized_deallocation >= 201309L)
-# error expected __cpp_sized_deallocation >= 201309L
+#  error expected __cpp_sized_deallocation >= 201309L
 #endif
 
 #include <new>
@@ -45,26 +45,23 @@
 
 #include "test_macros.h"
 
-int unsized_delete_called = 0;
+int unsized_delete_called         = 0;
 int unsized_delete_nothrow_called = 0;
-int sized_delete_called = 0;
+int sized_delete_called           = 0;
 
-void operator delete[](void* p) TEST_NOEXCEPT
-{
-    ++unsized_delete_called;
-    std::free(p);
+void operator delete[](void* p) TEST_NOEXCEPT {
+  ++unsized_delete_called;
+  std::free(p);
 }
 
-void operator delete[](void* p, const std::nothrow_t&) TEST_NOEXCEPT
-{
-    ++unsized_delete_nothrow_called;
-    std::free(p);
+void operator delete[](void* p, const std::nothrow_t&) TEST_NOEXCEPT {
+  ++unsized_delete_nothrow_called;
+  std::free(p);
 }
 
-void operator delete[](void* p, std::size_t) TEST_NOEXCEPT
-{
-    ++sized_delete_called;
-    std::free(p);
+void operator delete[](void* p, std::size_t) TEST_NOEXCEPT {
+  ++sized_delete_called;
+  std::free(p);
 }
 
 // NOTE: Use a class with a non-trivial destructor as the test type in order
@@ -76,19 +73,20 @@ void operator delete[](void* p, std::size_t) TEST_NOEXCEPT
 //   with two parameters is selected.
 // - Otherwise, it is unspecified which of the two deallocation functions is
 //   selected.
-struct A { ~A() {} };
-
-int main(int, char**)
-{
-    A* x = new A[3];
-    assert(0 == unsized_delete_called);
-    assert(0 == unsized_delete_nothrow_called);
-    assert(0 == sized_delete_called);
-
-    delete [] x;
-    assert(0 == unsized_delete_called);
-    assert(0 == unsized_delete_nothrow_called);
-    assert(1 == sized_delete_called);
-
-    return 0;
+struct A {
+  ~A() {}
+};
+
+int main(int, char**) {
+  A* x = new A[3];
+  assert(0 == unsized_delete_called);
+  assert(0 == unsized_delete_nothrow_called);
+  assert(0 == sized_delete_called);
+
+  delete[] x;
+  assert(0 == unsized_delete_called);
+  assert(0 == unsized_delete_nothrow_called);
+  assert(1 == sized_delete_called);
+
+  return 0;
 }
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align.pass.cpp
index 5d321f0828..854f5b40eb 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align.pass.cpp
@@ -34,15 +34,15 @@ void my_new_handler() {
 }
 
 int main(int, char**) {
-    test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
-        void* x = operator new(size, static_cast<std::align_val_t>(alignment));
-        assert(x != nullptr);
-        assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
-        operator delete(x, static_cast<std::align_val_t>(alignment));
-    });
+  test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
+    void* x = operator new(size, static_cast<std::align_val_t>(alignment));
+    assert(x != nullptr);
+    assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
+    operator delete(x, static_cast<std::align_val_t>(alignment));
+  });
 
-    // Test that the new handler is called if allocation fails
-    {
+  // Test that the new handler is called if allocation fails
+  {
 #ifndef TEST_HAS_NO_EXCEPTIONS
         std::set_new_handler(my_new_handler);
         try {
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp
index 3e5ba57411..5cf193a614 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp
@@ -34,20 +34,19 @@ void my_new_handler() {
 }
 
 int main(int, char**) {
-    test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
-        void* x = operator new(size, static_cast<std::align_val_t>(alignment), std::nothrow);
-        assert(x != nullptr);
-        assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
-        operator delete(x, static_cast<std::align_val_t>(alignment), std::nothrow);
-    });
+  test_with_interesting_alignments([](std::size_t size, std::size_t alignment) {
+    void* x = operator new(size, static_cast<std::align_val_t>(alignment), std::nothrow);
+    assert(x != nullptr);
+    assert(reinterpret_cast<std::uintptr_t>(x) % alignment == 0);
+    operator delete(x, static_cast<std::align_val_t>(alignment), std::nothrow);
+  });
 
-    // Test that the new handler is called and we return nullptr if allocation fails
-    {
-        std::set_new_handler(my_new_handler);
-        void* x = operator new(std::numeric_limits<std::size_t>::max(),
-                               static_cast<std::align_val_t>(32), std::nothrow);
-        assert(new_handler_called == 1);
-        assert(x == nullptr);
+  // Test that the new handler is called and we return nullptr if allocation fails
+  {
+    std::set_new_handler(my_new_handler);
+    void* x = operator new(std::numeric_limits<std::size_t>::max(), static_cast<std::align_val_t>(32), std::nothrow);
+    assert(new_handler_called == 1);
+    assert(x == nullptr);
     }
 
     // Test that a new expression constructs the right object
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete.pass.cpp
index fd52df451a..a4fb78a135 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete.pass.cpp
@@ -31,11 +31,11 @@
 // XFAIL: target={{.+}}-aix{{.*}}, target={{.+}}-zos{{.*}}
 
 #if !defined(__cpp_sized_deallocation)
-# error __cpp_sized_deallocation should be defined
+#  error __cpp_sized_deallocation should be defined
 #endif
 
 #if !(__cpp_sized_deallocation >= 201309L)
-# error expected __cpp_sized_deallocation >= 201309L
+#  error expected __cpp_sized_deallocation >= 201309L
 #endif
 
 #include <new>
@@ -45,41 +45,37 @@
 
 #include "test_macros.h"
 
-int unsized_delete_called = 0;
+int unsized_delete_called         = 0;
 int unsized_delete_nothrow_called = 0;
-int sized_delete_called = 0;
+int sized_delete_called           = 0;
 
-void operator delete(void* p) TEST_NOEXCEPT
-{
-    ++unsized_delete_called;
-    std::free(p);
+void operator delete(void* p) TEST_NOEXCEPT {
+  ++unsized_delete_called;
+  std::free(p);
 }
 
-void operator delete(void* p, const std::nothrow_t&) TEST_NOEXCEPT
-{
-    ++unsized_delete_nothrow_called;
-    std::free(p);
+void operator delete(void* p, const std::nothrow_t&) TEST_NOEXCEPT {
+  ++unsized_delete_nothrow_called;
+  std::free(p);
 }
 
-void operator delete(void* p, std::size_t) TEST_NOEXCEPT
-{
-    ++sized_delete_called;
-    std::free(p);
+void operator delete(void* p, std::size_t) TEST_NOEXCEPT {
+  ++sized_delete_called;
+  std::free(p);
 }
 
-int main(int, char**)
-{
-    int *x = new int(42);
-    DoNotOptimize(x);
-    assert(0 == unsized_delete_called);
-    assert(0 == unsized_delete_nothrow_called);
-    assert(0 == sized_delete_called);
-
-    delete x;
-    DoNotOptimize(x);
-    assert(0 == unsized_delete_called);
-    assert(1 == sized_delete_called);
-    assert(0 == unsized_delete_nothrow_called);
-
-    return 0;
+int main(int, char**) {
+  int* x = new int(42);
+  DoNotOptimize(x);
+  assert(0 == unsized_delete_called);
+  assert(0 == unsized_delete_nothrow_called);
+  assert(0 == sized_delete_called);
+
+  delete x;
+  DoNotOptimize(x);
+  assert(0 == unsized_delete_called);
+  assert(1 == sized_delete_called);
+  assert(0 == unsized_delete_nothrow_called);
+
+  return 0;
 }

This patch refactors the tests around aligned allocation and sized
deallocation to avoid relying on passing the -fsized-deallocation or
-faligned-allocation flags by default. Since both of these features are
enabled by default in >= C++14 mode, it now makes sense to make that
assumption in the test suite.

A notable exception is MinGW and some older compilers, where sized
deallocation is still not enabled by default. We treat that as a
"bug" in the test suite and we work around it by explicitly adding
-fsized-deallocation, but only under those configurations.
@ldionne ldionne force-pushed the review/fsized-deallocation branch from 3f86a7f to 2ac0fff Compare December 2, 2024 19:00
@ldionne ldionne merged commit 6cb339f into llvm:main Dec 6, 2024
60 of 63 checks passed
@ldionne ldionne deleted the review/fsized-deallocation branch December 6, 2024 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants