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

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 7 13:04:12 PDT 2019


rsmith added a comment.

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

> In D61165#1492781 <https://reviews.llvm.org/D61165#1492781>, @rsmith wrote:
>
> > (Those destructor calls are separate full-expressions that happen afterwards; see [intro.execution]/5.)
>
>
> Huh?  The destruction of temporaries at the end of a full-expression is definitely still part of the full-expression.  [class.temporary]p4: Temporary objects are destroyed as the last step in evaluating the full-expression that (lexically) contains the point where they were created.  [intro.execution] clarifies that all cases of individual initializers are full-expressions.


Sorry, I confused myself. You're right, of course. But I don't think that makes a material difference to anything else.

>> 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.

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.


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

https://reviews.llvm.org/D61165





More information about the cfe-commits mailing list