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

Fri May 3 10:59:34 PDT 2019

rjmccall added a comment.

>>> That's only true for subobjects of an enclosing aggregate before that aggregate's initialization is complete though, right? So it doesn't seem like that much of an inconsistency, just mimicking what we would be doing if an exception was thrown in, say, the body of the ctor before the object's initialization is completed.
>> 
>> Conceptually yes, but formally no.  The standard *could* write this rule as "all currently-initialized subobjects must be separately destroyed when an exception aborts initialization of the containing aggregate, including constructor bodies and aggregate initialization", but it doesn't actually do so; instead it has specific rules covering the behavior when an exception is thrown out of the body of a constructor, and those rules simply do not apply to aggregate initialization.
> 
> Right, I was just trying to draw an analogy. Can you be more specific about the inconsistency you mentioned above? What objects with static storage duration have to be destroyed when an exception is thrown? Just subobjects of static aggregates that had their initialization aborted by an exception? If so, that obviously doesn't seem inconsistent.

I see it as inconsistent that a destructor for a temporary can abort the initialization of an automatic object but not a static one.  I have seen programs that build idioms reliant on this kind of destructor ordering, e.g. as a way to commit / abort a "transaction".

>> Even if it did, though, that wouldn't tell us how to resolve this because this is fundamentally about when exactly the initialization of an object is "complete", which doesn't seem to be clearly defined in the standard.  There's a rule for when a *constructor* is complete, but among other things, not all initializations involve constructors.
> 
> Yeah, I agree that this is the fundamental problem here, and that the standard isn't much help. I'm trying to understand why you think this choice of semantics is better (or at least, good enough to make us hedge our bets here at the cost of keeping this feature simple and useful). I'm not seeing the aggregate initialization inconsistency you mentioned above. If the standard wanted us to call the destructor before the object's initialization was formally complete, then that seems like something they would mention.  Other implementations (well, GCC) seem to have made the opposite decision, which I think makes more intuitive sense.

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.

>>> So what should that path forward be here? I'd really like to get this crash fixed soon. If we want to consider a static local no_destroy dtor potentially-invoked in Sema if the initializer has a temporary with a throwing dtor, then we could do that, but it'd be unfortunate for 98/03 where I believe a dtor isn't noexcept by default, so we'd have to assume the worst. I guess it'd be easier to change our minds in the future if we treat the dtor as potentially-invoked, but I'm not really seeing the argument that we shouldn't just use this rule.
>> 
>> I think the simplest rule would be to say that the destructor must still be accessible for static or thread-local locals and that it'll be used in certain cases when initialization is aborted.
> 
> If we did this the source compat problem would be a lot less theoretical.

That's a good point, but there really can't be *that* many uses of this feature yet, especially in exceptions-enabled code.  We can fix semantic mistakes.

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

https://reviews.llvm.org/D61165