[PATCH] D53120: Implement sized deallocation for std::allocator and friends.

Eric Fiselier via Phabricator reviews at reviews.llvm.org
Fri Oct 12 08:40:32 PDT 2018


EricWF marked 3 inline comments as done.
EricWF added inline comments.


================
Comment at: include/new:328
+
+inline _LIBCPP_INLINE_VISIBILITY void __libcpp_deallocate(void* __ptr, size_t __size, size_t __align) {
+  _DeallocateCaller::__do_it(__ptr, __size, __align);
----------------
ckennelly wrote:
> Should this be specialized for the "don't know" case, rather than passing the size as 0?
> 
> For the common case (`std::allocator<T>::deallocate()`), we don't need to pay the cost of the branch.
Ack. I've fixed this by adding a `__libcpp_deallocate_unsized`.


================
Comment at: include/new:293-295
+    __do_call(__ptr);
+#else
+    return __do_call(__ptr, __size);
----------------
rsmith wrote:
> Any reason why one of these is `return`ed but the other is not?
I prefer explicit returns when inside conditional compilation blocks. I'll add return's above as well.


https://reviews.llvm.org/D53120





More information about the libcxx-commits mailing list