[libcxx-commits] [PATCH] D115730: [libc++] Add a helper class to write code with the strong exception guarantee

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In general LGTM, some small nits and questions.



================
Comment at: libcxx/include/CMakeLists.txt:143
   __debug
+  __detail/transaction.h
   __errc
----------------
Just curious what is the `__detail` directory intended to contain?


================
Comment at: libcxx/include/__detail/transaction.h:48
+//        return out;
+//    }
+//
----------------
I like this comment a lot!


================
Comment at: libcxx/include/__detail/transaction.h:51
+template <class _Rollback>
+struct __transaction {
+    __transaction() = default;
----------------
Somewhat curious, did you consider implementing `scope_exit` instead of `__transaction`?
https://en.cppreference.com/w/cpp/experimental/scope_exit/scope_exit

(I know that isn't `constexpr` (yet))


================
Comment at: libcxx/include/__detail/transaction.h:60
+
+    constexpr __transaction(__transaction&& __other)
+        : __rollback_(_VSTD::move(__other.__rollback_))
----------------



================
Comment at: libcxx/include/__detail/transaction.h:60
+
+    constexpr __transaction(__transaction&& __other)
+        : __rollback_(_VSTD::move(__other.__rollback_))
----------------
Mordante wrote:
> 
Should this function be conditionally `noexcept`?


================
Comment at: libcxx/include/__detail/transaction.h:75
+    _LIBCPP_HIDE_FROM_ABI
+    _LIBCPP_CONSTEXPR_AFTER_CXX17 ~__transaction() {
+        if (!__completed_)
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115730