On Thu, Jun 3, 2010 at 4:13 PM, Douglas Gregor <[email protected]> wrote: > > On Jun 3, 2010, at 1:39 PM, Eli Friedman wrote: > >> Author: efriedma >> Date: Thu Jun 3 15:39:03 2010 >> New Revision: 105408 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=105408&view=rev >> Log: >> Make sure to check the accessibility of and mark the destructor for the >> operand of a throw expression. Fixes PR7281. > > Cool. One comment below. > >> Added: >> cfe/trunk/test/CodeGenCXX/throw-expression-dtor.cpp >> Modified: >> cfe/trunk/lib/Sema/SemaExprCXX.cpp >> cfe/trunk/test/CXX/class.access/p4.cpp >> >> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=105408&r1=105407&r2=105408&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Thu Jun 3 15:39:03 2010 >> @@ -458,11 +458,28 @@ >> return true; >> E = Res.takeAs<Expr>(); >> >> + // If the exception has class type, we need additional handling. >> + const RecordType *RecordTy = Ty->getAs<RecordType>(); >> + if (!RecordTy) >> + return false; >> + CXXRecordDecl *RD = cast<CXXRecordDecl>(RecordTy->getDecl()); >> + >> // If we are throwing a polymorphic class type or pointer thereof, >> // exception handling will make use of the vtable. >> - if (const RecordType *RecordTy = Ty->getAs<RecordType>()) >> - MarkVTableUsed(ThrowLoc, cast<CXXRecordDecl>(RecordTy->getDecl())); >> - >> + MarkVTableUsed(ThrowLoc, RD); >> + >> + // If the class has a non-trivial destructor, we must be able to call it. >> + if (RD->hasTrivialDestructor()) >> + return false; >> + >> + CXXDestructorDecl *Destructor = >> + const_cast<CXXDestructorDecl*>(RD->getDestructor(Context)); >> + if (!Destructor) >> + return false; >> + >> + MarkDeclarationReferenced(E->getExprLoc(), Destructor); >> + CheckDestructorAccess(E->getExprLoc(), Destructor, >> + PDiag(diag::err_access_dtor_temp) << Ty); >> return false; >> } > > Could we get a new diagnostic that talks about the destructor of the > exception object, rather than reusing err_access_dtor_temp?
Would something like "exception object of type 'test16::A' has private destructor" be okay? >> >> Modified: cfe/trunk/test/CXX/class.access/p4.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.access/p4.cpp?rev=105408&r1=105407&r2=105408&view=diff >> ============================================================================== >> --- cfe/trunk/test/CXX/class.access/p4.cpp (original) >> +++ cfe/trunk/test/CXX/class.access/p4.cpp Thu Jun 3 15:39:03 2010 >> @@ -420,3 +420,9 @@ >> template class B<int>; // expected-note {{in instantiation}} >> template class B<long>; // expected-note 4 {{in instantiation}} >> } >> + >> +// PR7281 >> +namespace test16 { >> + class A { ~A(); }; // expected-note {{declared private here}} >> + void b() { throw A(); } // expected-error{{temporary of type 'test16::A' >> has private destructor}} >> +} >> >> Added: cfe/trunk/test/CodeGenCXX/throw-expression-dtor.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/throw-expression-dtor.cpp?rev=105408&view=auto >> ============================================================================== >> --- cfe/trunk/test/CodeGenCXX/throw-expression-dtor.cpp (added) >> +++ cfe/trunk/test/CodeGenCXX/throw-expression-dtor.cpp Thu Jun 3 15:39:03 >> 2010 >> @@ -0,0 +1,14 @@ >> +// RUN: %clang_cc1 %s -emit-llvm-only -verify >> +// PR7281 >> + >> +class A { >> +public: >> + ~A(); >> +}; >> +class B : public A { >> + void ice_throw(); >> +}; >> +void B::ice_throw() { >> + throw *this; >> +} >> + > > I guess this is just testing that CodeGen doesn't crash, right? Could we > check the generated IR to ensure that a pointer to the destructor is being > put into the right place as part of the throw statement? I think that's sufficiently covered by test/CodeGenCXX/eh.cpp, and I don't really know what we should be generating well enough to verify that it's working. -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
