-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[libc++] Don't use std::allocator inside <any> #161061
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
Conversation
3ecc3cb
to
f1bfc5b
Compare
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesThere isn't much of a reason to use the allocator abstraction facilities inside Full diff: https://github.com/llvm/llvm-project/pull/161061.diff 3 Files Affected:
diff --git a/libcxx/include/any b/libcxx/include/any
index 89bf3cf1f7df0..c1c45c151d281 100644
--- a/libcxx/include/any
+++ b/libcxx/include/any
@@ -84,10 +84,8 @@ namespace std {
# include <__cxx03/__config>
#else
# include <__config>
-# include <__memory/allocator.h>
-# include <__memory/allocator_destructor.h>
-# include <__memory/allocator_traits.h>
-# include <__memory/unique_ptr.h>
+# include <__memory/construct_at.h>
+# include <__new/allocate.h>
# include <__type_traits/add_cv_quals.h>
# include <__type_traits/add_pointer.h>
# include <__type_traits/aligned_storage.h>
@@ -103,6 +101,7 @@ namespace std {
# include <__type_traits/remove_cv.h>
# include <__type_traits/remove_cvref.h>
# include <__type_traits/remove_reference.h>
+# include <__utility/exception_guard.h>
# include <__utility/forward.h>
# include <__utility/in_place.h>
# include <__utility/move.h>
@@ -339,22 +338,14 @@ struct _SmallHandler {
template <class... _Args>
_LIBCPP_HIDE_FROM_ABI static _Tp& __create(any& __dest, _Args&&... __args) {
- typedef allocator<_Tp> _Alloc;
- typedef allocator_traits<_Alloc> _ATraits;
- _Alloc __a;
- _Tp* __ret = static_cast<_Tp*>(static_cast<void*>(&__dest.__s_.__buf));
- _ATraits::construct(__a, __ret, std::forward<_Args>(__args)...);
+ auto __ret = std::__construct_at(reinterpret_cast<_Tp*>(&__dest.__s_.__buf), std::forward<_Args>(__args)...);
__dest.__h_ = &_SmallHandler::__handle;
return *__ret;
}
private:
_LIBCPP_HIDE_FROM_ABI static void __destroy(any& __this) {
- typedef allocator<_Tp> _Alloc;
- typedef allocator_traits<_Alloc> _ATraits;
- _Alloc __a;
- _Tp* __p = static_cast<_Tp*>(static_cast<void*>(&__this.__s_.__buf));
- _ATraits::destroy(__a, __p);
+ std::__destroy_at(reinterpret_cast<_Tp*>(&__this.__s_.__buf));
__this.__h_ = nullptr;
}
@@ -406,26 +397,20 @@ struct _LargeHandler {
template <class... _Args>
_LIBCPP_HIDE_FROM_ABI static _Tp& __create(any& __dest, _Args&&... __args) {
- typedef allocator<_Tp> _Alloc;
- typedef allocator_traits<_Alloc> _ATraits;
- typedef __allocator_destructor<_Alloc> _Dp;
- _Alloc __a;
- unique_ptr<_Tp, _Dp> __hold(_ATraits::allocate(__a, 1), _Dp(__a, 1));
- _Tp* __ret = __hold.get();
- _ATraits::construct(__a, __ret, std::forward<_Args>(__args)...);
- __dest.__s_.__ptr = __hold.release();
+ _Tp* __ptr = static_cast<_Tp*>(std::__libcpp_allocate<_Tp>(__element_count(1)));
+ std::__exception_guard __guard([&] { std::__libcpp_deallocate<_Tp>(__ptr, __element_count(1)); });
+ std::__construct_at(__ptr, std::forward<_Args>(__args)...);
+ __guard.__complete();
+ __dest.__s_.__ptr = __ptr;
__dest.__h_ = &_LargeHandler::__handle;
- return *__ret;
+ return *__ptr;
}
private:
_LIBCPP_HIDE_FROM_ABI static void __destroy(any& __this) {
- typedef allocator<_Tp> _Alloc;
- typedef allocator_traits<_Alloc> _ATraits;
- _Alloc __a;
_Tp* __p = static_cast<_Tp*>(__this.__s_.__ptr);
- _ATraits::destroy(__a, __p);
- _ATraits::deallocate(__a, __p, 1);
+ std::__destroy_at(__p);
+ std::__libcpp_deallocate<_Tp>(__p, __element_count(1));
__this.__h_ = nullptr;
}
@@ -613,6 +598,11 @@ _LIBCPP_POP_MACROS
# include <type_traits>
# include <variant>
# endif
+
+# if !defined(_LIBCPP_REMOVE_TRANSITIVE_INCLUDES) & _LIBCPP_STD_VER <= 23
+# include <cstring>
+# include <limits>
+# endif
#endif // __cplusplus < 201103L && defined(_LIBCPP_USE_FROZEN_CXX03_HEADERS)
#endif // _LIBCPP_ANY
diff --git a/libcxx/test/libcxx/transitive_includes/cxx26.csv b/libcxx/test/libcxx/transitive_includes/cxx26.csv
index 81c8c41d88756..d047b29b63cc6 100644
--- a/libcxx/test/libcxx/transitive_includes/cxx26.csv
+++ b/libcxx/test/libcxx/transitive_includes/cxx26.csv
@@ -14,9 +14,7 @@ algorithm ratio
algorithm tuple
algorithm version
any cstdint
-any cstring
any initializer_list
-any limits
any typeinfo
any version
array cctype
diff --git a/libcxx/test/libcxx/utilities/any/allocator.pass.cpp b/libcxx/test/libcxx/utilities/any/allocator.pass.cpp
index eab3ca8826493..34827ff7acb32 100644
--- a/libcxx/test/libcxx/utilities/any/allocator.pass.cpp
+++ b/libcxx/test/libcxx/utilities/any/allocator.pass.cpp
@@ -17,111 +17,67 @@
#include <any>
#include <cassert>
#include <cstddef>
-#include <memory>
#include <new>
-#include <type_traits>
-#include <utility>
-#include "test_macros.h"
+// Make sure we don't fit in std::any's SBO
+int allocated_count = 0;
+int constructed_count = 0;
+struct Large {
+ Large() { ++constructed_count; }
-// Make sure we don't fit in std::any's SBO
-struct Large { char big[sizeof(std::any) + 1]; };
+ Large(const Large&) { ++constructed_count; }
-// Make sure we fit in std::any's SBO
-struct Small { };
-
-bool Large_was_allocated = false;
-bool Large_was_constructed = false;
-bool Large_was_destroyed = false;
-bool Large_was_deallocated = false;
-
-bool Small_was_constructed = false;
-bool Small_was_destroyed = false;
-
-template <>
-struct std::allocator<Large> {
- using value_type = Large;
- using size_type = std::size_t;
- using difference_type = std::ptrdiff_t;
- using propagate_on_container_move_assignment = std::true_type;
- using is_always_equal = std::true_type;
-
- Large* allocate(std::size_t n) {
- Large_was_allocated = true;
- return static_cast<Large*>(::operator new(n * sizeof(Large)));
- }
+ ~Large() { --constructed_count; }
- template <typename... Args>
- void construct(Large* p, Args&&... args) {
- new (p) Large(std::forward<Args>(args)...);
- Large_was_constructed = true;
- }
+ char big[sizeof(std::any) + 1];
- void destroy(Large* p) {
- p->~Large();
- Large_was_destroyed = true;
+ static void* operator new(size_t n) {
+ ++allocated_count;
+ return ::operator new(n);
}
- void deallocate(Large* p, std::size_t) {
- Large_was_deallocated = true;
- return ::operator delete(p);
+ static void operator delete(void* ptr) {
+ --allocated_count;
+ ::operator delete(ptr);
}
};
-template <>
-struct std::allocator<Small> {
- using value_type = Small;
- using size_type = std::size_t;
- using difference_type = std::ptrdiff_t;
- using propagate_on_container_move_assignment = std::true_type;
- using is_always_equal = std::true_type;
-
- Small* allocate(std::size_t) {
- assert(false);
- return nullptr;
- }
+// Make sure we fit in std::any's SBO
+struct Small {
+ Small() { ++constructed_count; }
- template <typename... Args>
- void construct(Small* p, Args&&... args) {
- new (p) Small(std::forward<Args>(args)...);
- Small_was_constructed = true;
- }
+ Small(const Small&) { ++constructed_count; }
+
+ ~Small() { --constructed_count; }
- void destroy(Small* p) {
- p->~Small();
- Small_was_destroyed = true;
+ static void* operator new(size_t n) {
+ ++allocated_count;
+ return ::operator new(n);
}
- void deallocate(Small*, std::size_t) { assert(false); }
+ static void operator delete(void* ptr) {
+ --allocated_count;
+ ::operator delete(ptr);
+ }
};
int main(int, char**) {
// Test large types
{
- {
- std::any a = Large();
- (void)a;
-
- assert(Large_was_allocated);
- assert(Large_was_constructed);
- }
-
- assert(Large_was_destroyed);
- assert(Large_was_deallocated);
+ [[maybe_unused]] std::any a = Large();
+ assert(constructed_count == 1);
}
+ assert(allocated_count == 0);
+ assert(constructed_count == 0);
// Test small types
{
- {
- std::any a = Small();
- (void)a;
-
- assert(Small_was_constructed);
- }
-
- assert(Small_was_destroyed);
+ [[maybe_unused]] std::any a = Small();
+ assert(constructed_count == 1);
}
+ assert(allocated_count == 0);
+ assert(constructed_count == 0);
return 0;
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with comments. Thanks for fixing this!
f1bfc5b
to
61fd754
Compare
There isn't much of a reason to use the allocator abstraction facilities inside
<any>
. We can improve compile times a bit by avoiding them and using__libcpp_{,de}allocate
directly. IMO this also significantly improves readability here.