[libcxx-commits] [libcxx] 39c8795 - [libc++] Use allocator_traits to consistently allocate/deallocate/construct/destroy objects in std::any

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Tue Sep 15 08:05:06 PDT 2020


Author: Marshall Clow
Date: 2020-09-15T11:04:59-04:00
New Revision: 39c8795141703a7d8313b2448d9d34e856df0b85

URL: https://github.com/llvm/llvm-project/commit/39c8795141703a7d8313b2448d9d34e856df0b85
DIFF: https://github.com/llvm/llvm-project/commit/39c8795141703a7d8313b2448d9d34e856df0b85.diff

LOG: [libc++] Use allocator_traits to consistently allocate/deallocate/construct/destroy objects in std::any

https://llvm.org/PR45099 notes (correctly) that we're inconsistent in memory
allocation in `std::any`. We allocate memory with `std::allocator<T>::allocate`,
construct with placement new, destroy by calling the destructor directly, and
deallocate by calling `delete`. Most of those are customizable by the user,
but in different ways.

The standard is silent on how these things are to be accomplished.
This patch makes it so we use `allocator_traits<allocator<T>>` for all
of these operations (allocate, construct, destruct, deallocate).
This is, at least, consistent.

Fixes https://llvm.org/PR45099.

Differential Revision: https://reviews.llvm.org/D81133

Added: 
    libcxx/test/libcxx/utilities/any/allocator.pass.cpp

Modified: 
    libcxx/include/any

Removed: 
    


################################################################################
diff  --git a/libcxx/include/any b/libcxx/include/any
index 36b07c9d7e75..7546f3124877 100644
--- a/libcxx/include/any
+++ b/libcxx/include/any
@@ -82,7 +82,6 @@ namespace std {
 
 #include <experimental/__config>
 #include <memory>
-#include <new>
 #include <typeinfo>
 #include <type_traits>
 #include <cstdlib>
@@ -368,7 +367,11 @@ namespace __any_imp
     template <class ..._Args>
     _LIBCPP_INLINE_VISIBILITY
     static _Tp& __create(any & __dest, _Args&&... __args) {
-        _Tp* __ret = ::new (static_cast<void*>(&__dest.__s.__buf)) _Tp(_VSTD::forward<_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, _VSTD::forward<_Args>(__args)...);
         __dest.__h = &_SmallHandler::__handle;
         return *__ret;
     }
@@ -376,8 +379,11 @@ namespace __any_imp
   private:
     _LIBCPP_INLINE_VISIBILITY
     static void __destroy(any & __this) {
-        _Tp & __value = *static_cast<_Tp *>(static_cast<void*>(&__this.__s.__buf));
-        __value.~_Tp();
+        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);
         __this.__h = nullptr;
     }
 
@@ -445,10 +451,12 @@ namespace __any_imp
     _LIBCPP_INLINE_VISIBILITY
     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(__a.allocate(1), _Dp(__a, 1));
-        _Tp* __ret = ::new ((void*)__hold.get()) _Tp(_VSTD::forward<_Args>(__args)...);
+        unique_ptr<_Tp, _Dp> __hold(_ATraits::allocate(__a, 1), _Dp(__a, 1));
+        _Tp * __ret = __hold.get();
+        _ATraits::construct(__a, __ret, _VSTD::forward<_Args>(__args)...);
         __dest.__s.__ptr = __hold.release();
         __dest.__h = &_LargeHandler::__handle;
         return *__ret;
@@ -458,7 +466,12 @@ namespace __any_imp
 
     _LIBCPP_INLINE_VISIBILITY
     static void __destroy(any & __this){
-        delete static_cast<_Tp*>(__this.__s.__ptr);
+        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);
         __this.__h = nullptr;
     }
 

diff  --git a/libcxx/test/libcxx/utilities/any/allocator.pass.cpp b/libcxx/test/libcxx/utilities/any/allocator.pass.cpp
new file mode 100644
index 000000000000..c6800eb832bd
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/any/allocator.pass.cpp
@@ -0,0 +1,136 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14
+
+// <any>
+
+// Check that we're consistently using std::allocator_traits to
+// allocate/deallocate/construct/destroy objects in std::any.
+// See https://llvm.org/PR45099 for details.
+
+#include <any>
+#include <cassert>
+#include <cstddef>
+#include <memory>
+#include <type_traits>
+#include <utility>
+
+#include "test_macros.h"
+
+
+// Make sure we don't fit in std::any's SBO
+struct Large { char big[sizeof(std::any) + 1]; };
+
+// 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_allocated = false;
+bool Small_was_constructed = false;
+bool Small_was_destroyed = false;
+bool Small_was_deallocated = false;
+
+namespace std {
+  template <>
+  struct allocator<Large> {
+    using value_type = Large;
+    using size_type = std::size_t;
+    using 
diff erence_type = std::ptr
diff _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));
+    }
+
+    template <typename ...Args>
+    void construct(Large* p, Args&& ...args) {
+      new (p) Large(std::forward<Args>(args)...);
+      Large_was_constructed = true;
+    }
+
+    void destroy(Large* p) {
+      p->~Large();
+      Large_was_destroyed = true;
+    }
+
+    void deallocate(Large* p, std::size_t) {
+      Large_was_deallocated = true;
+      return ::operator delete(p);
+    }
+  };
+
+  template <>
+  struct allocator<Small> {
+    using value_type = Small;
+    using size_type = std::size_t;
+    using 
diff erence_type = std::ptr
diff _t;
+    using propagate_on_container_move_assignment = std::true_type;
+    using is_always_equal = std::true_type;
+
+    Small* allocate(std::size_t n) {
+      Small_was_allocated = true;
+      return static_cast<Small*>(::operator new(n));
+    }
+
+    template <typename ...Args>
+    void construct(Small* p, Args&& ...args) {
+      new (p) Small(std::forward<Args>(args)...);
+      Small_was_constructed = true;
+    }
+
+    void destroy(Small* p) {
+      p->~Small();
+      Small_was_destroyed = true;
+    }
+
+    void deallocate(Small* p, std::size_t) {
+      Small_was_deallocated = true;
+      return ::operator delete(p);
+    }
+  };
+} // end namespace std
+
+
+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);
+  }
+
+  // Test small types
+  {
+    {
+      std::any a = Small();
+      (void)a;
+
+      assert(!Small_was_allocated);
+      assert(Small_was_constructed);
+    }
+
+    assert(Small_was_destroyed);
+    assert(!Small_was_deallocated);
+  }
+
+  return 0;
+}


        


More information about the libcxx-commits mailing list