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

> 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 :)
>
>
> If by "constructed" you mean "a constructor has finished", then we need to split this hair. Consider:
>
>   struct B { ~B(); };
>   struct A {
>     A() {}
>     A(B &&) : A() { throw 0; }
>   };
>   void f() {
>     static A a = B();
>   }
>
>
> At the point when the exception is thrown, a constructor for `a` has completed, but its initialization is not complete.
>
> [except.ctor]/3 and /4 lay out the rules:
>
> """
>  If the initialization or destruction of an object other than by delegating constructor is terminated by an
>  exception, the destructor is invoked for each of the object’s direct subobjects and, for a complete object,
>  virtual base class subobjects, whose initialization has completed (9.3) and whose destructor has not yet begun
>  execution, except that in the case of destruction, the variant members of a union-like class are not destroyed.
>  [Note: If such an object has a reference member that extends the lifetime of a temporary object, this ends
>  the lifetime of the reference member, so the lifetime of the temporary object is effectively not extended.
>  — end note] The subobjects are destroyed in the reverse order of the completion of their construction. Such
>  destruction is sequenced before entering a handler of the function-try-block of the constructor or destructor,
>  if any.
>
> If the compound-statement of the function-body of a delegating constructor for an object exits via an
>  exception, the object’s destructor is invoked. Such destruction is sequenced before entering a handler of the
>  function-try-block of a delegating constructor for that object, if any.
>  """
>
> The above wording seems to suggest that the initialization of an object of class type completes when its outermost constructor finishes (at least for the case of initialization by constructor). And indeed all the other wording I can find that has some bearing on when an object is deemed initialized suggests that option 1 is correct, and in general that the initialization of a variable is complete when the initialization full-expression ends, which is before the destructors for any temporaries run. (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.

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


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

https://reviews.llvm.org/D61165