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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.

An interesting thought occurred to me last night. Let's say we have the following sandwich of code:

  A // -fexceptions
  B // -fno-exceptions
  C // -fexceptions

Now assume that C throws an exception. The current behavior is that the exception will propagate through B (where it can't be caught) and then potentially be caught inside A. I don't know how common the full sandwich is, however a very common example of B and C are `B = any-code-compiled-without-exceptions` and `C = libc++.dylib on most platforms`. I would presume that this situation does arise in the wild.

We looked at the code generation in `-fno-exceptions` and the `B` slice of the sandwich simply does not contain any cleanup code (and hence no call to `__transaction`'s destructor). If it did, the fact that we have a `_LIBCPP_ASSERT` inside the destructor would be a behavioral change where instead of letting the exception propagate freely through `B` (and potentially being caught higher up), we would now instead turn it into a hard error via `__libcpp_verbose_abort`.

We also discussed the possibility of not making this change at all so that the destructor can be called to clean things up if needed. However, since Clang already does not generate any cleanup code when `-fno-exceptions` is used, it looks like the choice not to provide the usual exception safety guarantees in `-fno-exceptions` code has already been made, and we're just following suite.

@philnik Can you please summarize this in a comment somewhere in `__transaction`? I think this is useful for posterity since it's not exactly obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133661