[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

Tue Apr 30 15:37:36 PDT 2019

jfb added a comment.

In D61165#1479937 <https://reviews.llvm.org/D61165#1479937>, @rjmccall wrote:

> Are you sure these are the right semantics for `nodestroy`?  I think there's a reasonable argument that we should not destroy previous elements of a `nodestroy` array just because an element constructor throws.  It's basically academic for true globals because the exception will terminate the process anyway, and even for `thread_local`s and `static` locals (where I believe the exception is theoretically recoverable) it's at least arguable that we should either decline to destroy (possibly causing leaks) or simply call `std::terminate`.

I think `std::terminate` makes the most sense. Getting teardown correctly is always tricky, and I'm willing to bet that teardown caused by an exception in construction of an array is even harder and done wrong.

================
Comment at: clang/include/clang/Basic/AttrDocs.td:3906
+    [[clang::no_destroy]]
+    static only_no_destroy array[10]; // error, only_no_destroy has a private destructor.
+
----------------
You probably want a `try`/`catch` here to illustrate why exceptions can matter.

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

https://reviews.llvm.org/D61165