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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__utility/transaction.h:55
+    _LIBCPP_HIDE_FROM_ABI
+    _LIBCPP_CONSTEXPR explicit __transaction(_Rollback __rollback)
+        : __rollback_(_VSTD::move(__rollback))
----------------
Mordante wrote:
> I see a lot of different `constexpr` macros in this class
> `_LIBCPP_CONSTEXPR`, `_LIBCPP_CONSTEXPR_AFTER_CXX11` and `_LIBCPP_CONSTEXPR_AFTER_CXX17`. Looking at this I expect the class not be usable as constexpr before C++20, due to the `constexpr` macro for the destructor.
This is presumably due to a style guideline that I made up ;) a while ago: "use the most aggressive constexpr macro possible for each place in internal code," so that if we ever drop support for C++03 (or Clang gains support for `constexpr` as an extension in C++03 mode), we can replace the absolute maximum number of uses of `_LIBCPP_CONSTEXPR` in one fell swoop. I.e. I think this is fine.

But, given the intended usage of this class... do we need it to be constexpr at all? Certainly not for `call_once` or `uninitialized_copy_n`. @ldionne, could you add some sample usages (in places like <algorithm> and the various containers, maybe) that show why constexprness is a good idea? Or if no great examples are conspicuous, I'd recommend removing the constexpr keywords entirely for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115730