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

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

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

> Hmm.  You know, there's another case where the destructor can be called even for a non-array: if constructing the object requires a temporary, I believe an exception thrown from that temporary's destructor is supposed to go back and destroy the variable.  (This is, sadly, not entirely clear under the standard since the object is not automatic.)  Now, Clang does not actually get this correct — we abort the construction, but we don't destroy the variable — but (1) we should fix that someday and (2) in the meantime, we shouldn't implement something which will be a problem when we go to fix that.


Just to be clear, you're thinking of something like this:

  struct S {
    S();
    ~S() noexcept(false) { throw 0; }
  };
  
  struct G {
    G(const S &) noexcept;
    ~G();
  };
  
  int main() { G g = S(); }

Which clang just generates straight-line code for, even in `-fexceptions` mode. This seems wrong.

  call void @_ZN1SC1Ev(%struct.S* %2)
  call void @_ZN1GC1ERK1S(%struct.G* %1, %struct.S* dereferenceable(1) %2) #4
  call void @_ZN1SD1Ev(%struct.S* %2)
  call void @_ZN1GD1Ev(%struct.G* %1) #4

It seems like the most common sense interpretation here is to just treat the initialization of `G` as completed at the point when the constructor finishes (this appears to be what GCC implements: https://wandbox.org/permlink/R3C9pPhoT4efAdL1). So if it was static it would just get destroyed at exit-time, and therefore should be disable-able with `no_destroy`. If the standard implies that we should be doing something else, then we should do that, but I can't seem to find any reference to the rule you're describing.


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

https://reviews.llvm.org/D61165