[libcxx-commits] [PATCH] D154017: Cleanup __uninitialized_temporary_buffer internals.

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added inline comments.


================
Comment at: libcxx/include/__memory/uninitialized_buffer.h:51-53
+ * 1. No initialization or destruction of elements:
+ *    It is the callers responsibility to manage both construction and destruction of the elements created
+ *    in the buffer.
----------------
ldionne wrote:
> The ability to have a non-trivial destructor was the reason why we created this class and refactored the existing uses of `get_temporary_buffer()` in the first place. We can't remove this functionality, this is needed for implementing the GCD backend in the PSTL.
We can cross that bridge and add that functionality when it's needed.

As it stands now, the features your referring to are both unused and broken. 
I'm happy to have a more throughout conversation on API design later, but right now we're trying to fix forward to avoid having to revert even further.

It would be better & safer to check in functionality only once it's going to be used. Otherwise, we get untested dead code with bugs in it waiting to be discovered.


================
Comment at: libcxx/include/__memory/uninitialized_buffer.h:61
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_CFI __uninitialized_buffer_t<_Tp>
+__maybe_allocate_uninitialized_buffer(size_t __count) {
+  _Tp* __ptr = static_cast<_Tp*>(std::__libcpp_allocate(sizeof(_Tp) * __count, _LIBCPP_ALIGNOF(_Tp), nothrow));
----------------
ldionne wrote:
> `__try_allocate_uninitialized_buffer`?
I have no preference on the name, but `try` to me implies that it can fail and throw. Where `maybe` seems like failing is a non-exceptional condition, as it is here.
Your call.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154017/new/

https://reviews.llvm.org/D154017