[PATCH] D137070: [clang][Interp] Support destructors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM assuming no surprises with the new test request.



================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1915-1916
+
+    if (const CXXDestructorDecl *Dtor = ElemRecord->getDestructor();
+        Dtor && !Dtor->isTrivial()) {
+      for (size_t I = 0, E = Desc->getNumElems(); I != E; ++I) {
----------------
tbaeder wrote:
> aaron.ballman wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > `isTrivial()` only works once the class has been fully built up by Sema IIRC; we should have a test case for that situation.
> > > Are you saying that `isTrivial()` cannot be used like this, or just that it can, but needs a test case to ensure that this is true?
> > > 
> > > Also, how would such a test case look like?
> > `Sema::DeclareImplicitDestructor()` decides whether the destructor is trivial or not, and that is based on information that the class collects as the class is being declared. While the class is being parsed, the `DeclarationData` for the class is updated as we go and we use that to decide if we need the destructor, whether it's trivial, etc. So it's possible for us to have not seen a part of the class yet that would cause the special member function to be (non)trivial and so asking the method "are you trivial" may give a different answer depending on when the question is asked.
> > 
> > In terms of a test case, I think it would be trying to hit one of these cases http://eel.is/c++draft/class.mem#class.dtor-8 by using a constexpr function that needs to be evaluated before we get to something that causes the dtor to no longer be trivial.
> Hm, I can't come up with a reproducer for this. The class of a member variable must be fully defined when the member is declared, so I can't forward-declare it and then introduce a non-trivial destructor later. And as soon as I add a destructor declaration (and try to define i later), the destructor is automatically not trivial anymore.
Yeah, I'm struggling to make a test case as well, so let's move on.


================
Comment at: clang/test/AST/Interp/cxx20.cpp:439
+  static_assert(Foo::a.A == 0);
+
+
----------------
Another test case that I think would be interesting is with a static member that is not constexpr (to show it doesn't cause problems) and an out-of-line destructor just because it's a bit of an oddity: https://godbolt.org/z/YqrPMEEr4


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

https://reviews.llvm.org/D137070