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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D61165#1494001 <https://reviews.llvm.org/D61165#1494001>, @rsmith wrote:

> In D61165#1492830 <https://reviews.llvm.org/D61165#1492830>, @rjmccall wrote:
>
> > In D61165#1492781 <https://reviews.llvm.org/D61165#1492781>, @rsmith wrote:
> >
> > > For the purposes of this patch, I think that means we never need a destructor for the type of a `[[no_destroy]]` variable.
> >
> >
> > Arrays and other subobjects of an aggregate initializaton, unless applying the standard "literally" implies the obviously perverse result that we don't destroy subobjects in such cases.
>
>
> Yes, but you need the destructors for those subobjects as a side-condition of the initialization, irrespective of what kind of object that initialization is initializing. I don't think that's got anything to do with `[[no_destroy]]`. I think it remains the case that you never need a destructor for the type of a `[[no_destroy]]` variable.


Okay, so the rule should be that `[[no_destroy]]` never requires the destructor for the exact type of the variable, but it might require the destructors of sub-object types of a `static` local that uses aggregate initialization when exceptions are enabled, which necessarily includes the element type of an array.

> So far the opinion of folks on the core reflector has unanimously been that (1) is the right model. And I think that makes sense: it would be somewhat strange for the initialization of a complete object to be considered complete after temporaries are destroyed, but the initializations of its subobjects to be considered complete before the temporaries are destroyed.

Okay.  I'm not sure that's the decision I would make, but I can accept it, and it certainly simplifies the model in some ways.


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

https://reviews.llvm.org/D61165