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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
