[PATCH] D27358: [MS-ABI]V-base dtor called more than needed when throw happens in v-base ctor in window. Need add "complete object flag" check in eh cleanup code.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added a comment.

Hi Reid,

I know some problems(ms compatibility) when throw inside destructor.  I have not yet look like those problems.  I am new for clang.  I need sometime to catch up.  -:)

Thank you so much for your code review.  I had add new patch to address your suggestion.

Please take look.  Let me know if you need more info.

Thanks.
Jennifer



================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1530-1532
+    const CXXRecordDecl *ClassDecl =
+             cast<CXXConstructorDecl>(CGF.CurCodeDecl)->getParent();
+    if (ClassDecl != nullptr && ClassDecl->getNumVBases())
----------------
rnk wrote:
> These checks seem unnecessary. ForVirtualBase should never be true if there are no vbases, and the IsMostDerivedClass assert will catch it if not.
Yes, you are right.  I can either check here, or check if IsMostDerivedClass is nullptr return instead assertion inside EmitDtorCompleteObjectHandler.

As you know ForVirutalBase is set also for destructor.  But we only need this for ctor.  




================
Comment at: test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp:285
+  int var_0;
+  class_2() {};
+  virtual ~class_2() {
----------------
rnk wrote:
> Most of these can be declarations to reduce the output size.
Thank you!  I changed, see my new diff


================
Comment at: test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp:316
+  // WIN32: [[DTOR_VBASE]]
+  // WIN32: br label %[[SKIP_VBASE]]
+  // WIN32: [[SKIP_VBASE]]
----------------
rnk wrote:
> Check for a call to the class_2 destructor here
Thank you so much.  I changed see my new diff.


https://reviews.llvm.org/D27358