[PATCH] D77081: [MS] Mark vbase dtors ref'd when ref'ing dtor

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/include/clang/AST/DeclCXX.h:959-963
+  /// Indicates if the complete destructor has been implicitly declared
+  /// yet. Only relevant in the Microsoft C++.
+  bool definedImplicitCompleteDestructor() const {
+    return data().DefinedImplicitCompleteDestructor;
+  }
----------------
"declared" in comment but "defined" in function name. Which is it?

I wonder if perhaps the right answer is neither: if we had separate `Decl`s for the complete and base destructors, this would be tracked by the "used" bit on the complete object destructor. So perhaps `isCompleteDestructorUsed` and `setCompleteDestructorUsed` would make sense? (We could consider tracking this on the destructor instead of here, but I suppose tracking it here gives us the serialization and merging logic for free.)


================
Comment at: clang/include/clang/Sema/Sema.h:5562-5565
+  /// Define an implicit complete constructor for classes with vbases in the MS
+  /// ABI.
+  void DefineImplicitCompleteDestructor(SourceLocation CurrentLocation,
+                                        CXXDestructorDecl *Dtor);
----------------
Following the prior comment, naming this `MarkCompleteDestructorUsed` might make sense.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:16008-16013
+            // In the MS ABI, the complete destructor is implicitly defined,
+            // even if the base destructor is user defined.
+            CXXRecordDecl *Parent = Destructor->getParent();
+            if (Parent->getNumVBases() > 0 &&
+                !Parent->definedImplicitCompleteDestructor())
+              DefineImplicitCompleteDestructor(Loc, Destructor);
----------------
Can we avoid doing this when we know the call is to a base subobject destructor or uses virtual dispatch? Should we? (I mean I guess ideally we'd change this function to take a `GlobalDecl` instead of a `FunctionDecl*`, but that seems like a lot of work.)

How does MSVC behave?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77081