On Jun 3, 2010, at 4:25 PM, Eli Friedman wrote:
> On Thu, Jun 3, 2010 at 4:13 PM, Douglas Gregor <dgre...@apple.com> 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.
Yes, I see it tested in eh.cpp. Thanks!
- Doug
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
