Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++] Simplify when the sized global deallocations overloads are available #114667

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

philnik777
Copy link
Contributor

There doesn't seem to be much benefit in always providing declarations for the sized deallocations from C++14 onwards if the user explicitly passed -fno-sized-deallocation to disable them. This patch simplifies the declarations to be available exactly when the compiler expects sized deallocation functions to be available.

@philnik777 philnik777 requested a review from a team as a code owner November 2, 2024 13:42
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 2, 2024
@llvmbot
Copy link

llvmbot commented Nov 2, 2024

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

There doesn't seem to be much benefit in always providing declarations for the sized deallocations from C++14 onwards if the user explicitly passed -fno-sized-deallocation to disable them. This patch simplifies the declarations to be available exactly when the compiler expects sized deallocation functions to be available.


Full diff: https://github.com/llvm/llvm-project/pull/114667.diff

1 Files Affected:

  • (modified) libcxx/include/new (+4-16)
diff --git a/libcxx/include/new b/libcxx/include/new
index 290ad9e97f8ded..c7678b64c7d494 100644
--- a/libcxx/include/new
+++ b/libcxx/include/new
@@ -104,18 +104,6 @@ void  operator delete[](void* ptr, void*) noexcept;
 #endif
 
 #if defined(__cpp_sized_deallocation) && __cpp_sized_deallocation >= 201309L
-#  define _LIBCPP_HAS_LANGUAGE_SIZED_DEALLOCATION 1
-#else
-#  define _LIBCPP_HAS_LANGUAGE_SIZED_DEALLOCATION 0
-#endif
-
-#if _LIBCPP_STD_VER >= 14 || _LIBCPP_HAS_LANGUAGE_SIZED_DEALLOCATION
-#  define _LIBCPP_HAS_LIBRARY_SIZED_DEALLOCATION 1
-#else
-#  define _LIBCPP_HAS_LIBRARY_SIZED_DEALLOCATION 0
-#endif
-
-#if _LIBCPP_HAS_LIBRARY_SIZED_DEALLOCATION && _LIBCPP_HAS_LANGUAGE_SIZED_DEALLOCATION
 #  define _LIBCPP_HAS_SIZED_DEALLOCATION 1
 #else
 #  define _LIBCPP_HAS_SIZED_DEALLOCATION 0
@@ -214,7 +202,7 @@ inline constexpr destroying_delete_t destroying_delete{};
     _LIBCPP_NOALIAS;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete(void* __p) _NOEXCEPT;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete(void* __p, const std::nothrow_t&) _NOEXCEPT;
-#  if _LIBCPP_HAS_LIBRARY_SIZED_DEALLOCATION
+#  if _LIBCPP_HAS_SIZED_DEALLOCATION
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete(void* __p, std::size_t __sz) _NOEXCEPT;
 #  endif
 
@@ -223,7 +211,7 @@ _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete(void* __p, std::size_t __sz) _
     _LIBCPP_NOALIAS;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete[](void* __p) _NOEXCEPT;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete[](void* __p, const std::nothrow_t&) _NOEXCEPT;
-#  if _LIBCPP_HAS_LIBRARY_SIZED_DEALLOCATION
+#  if _LIBCPP_HAS_SIZED_DEALLOCATION
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete[](void* __p, std::size_t __sz) _NOEXCEPT;
 #  endif
 
@@ -233,7 +221,7 @@ _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete[](void* __p, std::size_t __sz)
 operator new(std::size_t __sz, std::align_val_t, const std::nothrow_t&) _NOEXCEPT _LIBCPP_NOALIAS;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete(void* __p, std::align_val_t) _NOEXCEPT;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete(void* __p, std::align_val_t, const std::nothrow_t&) _NOEXCEPT;
-#    if _LIBCPP_HAS_LIBRARY_SIZED_DEALLOCATION
+#    if _LIBCPP_HAS_SIZED_DEALLOCATION
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete(void* __p, std::size_t __sz, std::align_val_t) _NOEXCEPT;
 #    endif
 
@@ -243,7 +231,7 @@ operator new[](std::size_t __sz, std::align_val_t) _THROW_BAD_ALLOC;
 operator new[](std::size_t __sz, std::align_val_t, const std::nothrow_t&) _NOEXCEPT _LIBCPP_NOALIAS;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete[](void* __p, std::align_val_t) _NOEXCEPT;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete[](void* __p, std::align_val_t, const std::nothrow_t&) _NOEXCEPT;
-#    if _LIBCPP_HAS_LIBRARY_SIZED_DEALLOCATION
+#    if _LIBCPP_HAS_SIZED_DEALLOCATION
 _LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete[](void* __p, std::size_t __sz, std::align_val_t) _NOEXCEPT;
 #    endif
 #  endif

@ldionne
Copy link
Member

ldionne commented Nov 4, 2024

This makes sense to me, and I think the same should apply to aligned allocation as well (we should do that separately).

However, I'd like to get @EricWF 's thoughts on this since he was the one to introduce this 2-level dance for aligned allocation if I remember correctly.

Also, some CI failures are not flukes and should be addressed.

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.

3 participants