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

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington planned changes to this revision.
erik.pilkington added a comment.

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

> I believe at least one of the goals of `nodestroy` is to allow the type to potentially not provide a destructor at all, so if we're going to implicitly require the destructor anyway in certain situations, we should clearly document that, and we should be aware that we may be making the attribute less useful.
>
> Since I believe the dominant use-case here is a true global, does only requiring the destructor for arrays in the static-local case when exceptions are enabled at least make it acceptable to do proper access checking, or is that still a source-compatibility problem for existing clients?


It's hard to say exactly how bad the source compatibility problem is, we haven't been pushing on this much internally (yet), so I doubt we'll run into any issues. There was included in an open-source release though. If we only checked access when we needed it, the sequence of steps to actually break anything is pretty far: you'd have to have a type with a private dtor, used in a static local array, with the attribute, and be compiling with exceptions. I'd actually be quite surprised if we ran into any issues, so I guess we should do the right thing with access checking.

I'll update the patch and the docs.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61165