[cfe-commits] r105408 - in /cfe/trunk: lib/Sema/SemaExprCXX.cpp test/CXX/class.access/p4.cpp test/CodeGenCXX/throw-expression-dtor.cpp

[Douglas Gregor]

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?

> 
> 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?

	- Doug