[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#1490417 <https://reviews.llvm.org/D61165#1490417>, @erik.pilkington wrote:

> In D61165#1490168 <https://reviews.llvm.org/D61165#1490168>, @rjmccall wrote:
>
> > I think the intuitive rule is that initialization is complete when the full-expression performing the initialization is complete because that's the normal unit of sequencing.  Note that the standard does use both "constructed" and "initialized" in different contexts, although this might just be an editorial choice.
>
>
> I think it would be quite subtle if the standard was deliberately splitting this particular hair by implicitly creating a "constructed, but not initialized" state of an object, without calling that out anywhere :)


Well, but you're assuming that the standard is just using two different words for the same concept, often in close proximity.  That's probably against some canon of interpretation.

> As far as I see it, we have three options here:
> 
> 1. Claim that the object is formally initialized when the constructor returns
> 
>   This appears to be what GCC implements.

Outside of aggregate initialization, yes.  For aggregate initialization, GCC appears to treat the object as fully initialized as soon as its last member is constructed; until that point, no destructors are run for fully-constructed previous members, which is obviously a bug at some level.

I guess my constructed-vs-initialized rule would naturally suggest that exceptions thrown by full-expression destructors for the last member's initializer cause all the members to be individually destroyed without invoking the aggregate's destructor.  (Somewhat oddly, aggregates can have user-defined destructors, so this is a detectable difference.)  This is a little weird.

> 2. Claim that the object is formally initialized when the full-expression ends, and if a temporary throws don't call the destructor because the object isn't initialized.
> 
>   This is what Clang implements today, but seems wrong.

Yes, I acknowledged way upthread that this is a bug.

> 3. Claim that the object is formally initialized when the full-expression ends, but if a temporary throws call the destructor because the constructor ran.
> 
>   This seems weird to me. If destroying temporaries is an indivisible part of the initialization of an object, then we shouldn't be able to call the destructor, because the initialization of the object didn't succeed. (I mean, assuming there isn't a distinction between constructed and initialized)

I think drawing that distinction is a necessary precursor to applying my rule.

>> there really can't be *that* many uses of this feature yet, especially in exceptions-enabled code. We can fix semantic mistakes.
> 
> Yeah, if we assert rule 3 then we I guess we should just do this, rather than try to determine whether we need the dtor for the static local in Sema.
> 
> Anyways, I think I've layed out my thinking here as clearly as I can. If you still think that 3 is right here, then I'm happy to defer to you (although it would be quite nice if @rsmith chimed in too). I'm happy to implement whatever the right thing for CodeGen to do here is too.

I would also be interested in getting Richard's opinion on this.


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

https://reviews.llvm.org/D61165