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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

I'm not fond of the current state; it still has most of the semantics of the transaction instead of being a exception guard. I think with my suggestion we turn the class in a real exception guard and we never have the risk of "forgetting" to complete.



================
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) {
----------------
This really makes it a lot easier to review and maintain.


================
Comment at: libcxx/include/__utility/exception_guard.h:78
         __completed_ = true;
     }
 
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133661