[libcxx-commits] [PATCH] D133661: [libc++] Improve binary size when using __transaction

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik marked 2 inline comments as done.
philnik added inline comments.


================
Comment at: libcxx/include/__memory/uninitialized_algorithms.h:541-542
   auto __destruct_first = __first2;
-  try {
-#endif
+  auto __guard =
+      std::__make_exception_guard(_AllocatorDestroyRangeReverse<_Alloc, _Iter2>(__alloc, __destruct_first, __first2));
   while (__first1 != __last1) {
----------------
Mordante wrote:
> This really makes it a lot easier to review and maintain.
This still doesn't work, since it uses CTAD.


================
Comment at: libcxx/include/__utility/exception_guard.h:78
         __completed_ = true;
     }
 
----------------
Mordante wrote:
> When this class is intended to be an exception guard it doesn't need this function nor the `__completed_` member. Instead we can do something along the lines of:
> ```
> #ifndef _LIBCPP_NO_EXCEPTIONS
> #if _LIBCPP_STD_VER > 14
>   _LIBCPP_HIDE_FROM_ABI bool __need_rollback() { return uncaught_exceptions(); } 
> #else
>   _LIBCPP_HIDE_FROM_ABI bool __need_rollback() { return uncaught_exception(); } 
> #endif
> 
> 
>  _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__exception_guard() {
>     if(__need_rollback())
>       __rollback_();
>   }
> #endif 
> ```
> With some extra boilerplate to add `_LIBCPP_CONSTEXPR_SINCE_CXX20`.
> 
> Then we can even use most of this class when `_LIBCPP_NO_EXCEPTIONS` is defined. Then it just does an extra store of the never used `__rollback_` member.
This would break when running in a destructor though, right? Then `uncaught_exception()` might return true even if we don't actually want to roll back a transaction. That's the whole reason `uncaught_exceptions()` was added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133661