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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to