Diagnose uses of deleted destructors and inaccessible defaulted destructors.

We had two separate issues here: firstly, varions functions were assuming that
they did not need to perform semantic checks on trivial destructors (this is
not true in C++11, where a trivial destructor can nonetheless be private or
deleted), and a bunch of DiagnoseUseOfDecl calls were missing for uses of
destructors.

llvm-svn: 150866

clang/include/clang/AST/DeclCXX.h

@@ -1122,6 +1122,13 @@ public:
   // (C++ [class.dtor]p3)
   bool hasTrivialDestructor() const { return data().HasTrivialDestructor; }
 
+  // hasIrrelevantDestructor - Whether this class has a destructor which has no
+  // semantic effect. Any such destructor will be trivial, public, defaulted
+  // and not deleted.
+  bool hasIrrelevantDestructor() const {
+    return hasTrivialDestructor() && !hasUserDeclaredDestructor();
+  }
+
   // hasNonLiteralTypeFieldsOrBases - Whether this class has a non-literal or
   // volatile type non-static data member or base class.
   bool hasNonLiteralTypeFieldsOrBases() const {

clang/lib/Sema/SemaDeclCXX.cpp

@@ -3282,7 +3282,7 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
     CXXRecordDecl *FieldClassDecl = cast<CXXRecordDecl>(RT->getDecl());
     if (FieldClassDecl->isInvalidDecl())
       continue;
-    if (FieldClassDecl->hasTrivialDestructor())
+    if (FieldClassDecl->hasIrrelevantDestructor())
       continue;
 
     CXXDestructorDecl *Dtor = LookupDestructor(FieldClassDecl);
@@ -3293,6 +3293,7 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
                             << FieldType);
 
     MarkFunctionReferenced(Location, const_cast<CXXDestructorDecl*>(Dtor));
+    DiagnoseUseOfDecl(Dtor, Location);
   }
 
   llvm::SmallPtrSet<const RecordType *, 8> DirectVirtualBases;
@@ -3311,8 +3312,7 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
     // If our base class is invalid, we probably can't get its dtor anyway.
     if (BaseClassDecl->isInvalidDecl())
       continue;
-    // Ignore trivial destructors.
-    if (BaseClassDecl->hasTrivialDestructor())
+    if (BaseClassDecl->hasIrrelevantDestructor())
       continue;
 
     CXXDestructorDecl *Dtor = LookupDestructor(BaseClassDecl);
@@ -3325,6 +3325,7 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
                             << Base->getSourceRange());
     
     MarkFunctionReferenced(Location, const_cast<CXXDestructorDecl*>(Dtor));
+    DiagnoseUseOfDecl(Dtor, Location);
   }
   
   // Virtual bases.
@@ -3342,8 +3343,7 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
     // If our base class is invalid, we probably can't get its dtor anyway.
     if (BaseClassDecl->isInvalidDecl())
       continue;
-    // Ignore trivial destructors.
-    if (BaseClassDecl->hasTrivialDestructor())
+    if (BaseClassDecl->hasIrrelevantDestructor())
       continue;
 
     CXXDestructorDecl *Dtor = LookupDestructor(BaseClassDecl);
@@ -3353,6 +3353,7 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
                             << VBase->getType());
 
     MarkFunctionReferenced(Location, const_cast<CXXDestructorDecl*>(Dtor));
+    DiagnoseUseOfDecl(Dtor, Location);
   }
 }
 
@@ -8918,7 +8919,7 @@ void Sema::FinalizeVarWithDestructor(VarDecl *VD, const RecordType *Record) {
 
   CXXRecordDecl *ClassDecl = cast<CXXRecordDecl>(Record->getDecl());
   if (ClassDecl->isInvalidDecl()) return;
-  if (ClassDecl->hasTrivialDestructor()) return;
+  if (ClassDecl->hasIrrelevantDestructor()) return;
   if (ClassDecl->isDependentContext()) return;
 
   CXXDestructorDecl *Destructor = LookupDestructor(ClassDecl);
@@ -8927,6 +8928,7 @@ void Sema::FinalizeVarWithDestructor(VarDecl *VD, const RecordType *Record) {
                         PDiag(diag::err_access_dtor_var)
                         << VD->getDeclName()
                         << VD->getType());
+  DiagnoseUseOfDecl(Destructor, VD->getLocation());
 
   if (!VD->hasGlobalStorage()) return;
 

clang/lib/Sema/SemaExprCXX.cpp

@@ -637,8 +637,8 @@ ExprResult Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc, Expr *E,
   if (isPointer)
     return Owned(E);
 
-  // If the class has a non-trivial destructor, we must be able to call it.
-  if (RD->hasTrivialDestructor())
+  // If the class has a destructor, we must be able to call it.
+  if (RD->hasIrrelevantDestructor())
     return Owned(E);
 
   CXXDestructorDecl *Destructor
@@ -649,6 +649,7 @@ ExprResult Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc, Expr *E,
   MarkFunctionReferenced(E->getExprLoc(), Destructor);
   CheckDestructorAccess(E->getExprLoc(), Destructor,
                         PDiag(diag::err_access_dtor_exception) << Ty);
+  DiagnoseUseOfDecl(Destructor, E->getExprLoc());
   return Owned(E);
 }
 
@@ -1318,6 +1319,7 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal,
       CheckDestructorAccess(StartLoc, dtor, 
                             PDiag(diag::err_access_dtor)
                               << Context.getBaseElementType(AllocType));
+      DiagnoseUseOfDecl(dtor, StartLoc);
     }
   }
 
@@ -2064,7 +2066,7 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
           UsualArrayDeleteWantsSize = (OperatorDelete->getNumParams() == 2);
       }
 
-      if (!PointeeRD->hasTrivialDestructor())
+      if (!PointeeRD->hasIrrelevantDestructor())
         if (CXXDestructorDecl *Dtor = LookupDestructor(PointeeRD)) {
           MarkFunctionReferenced(StartLoc,
                                     const_cast<CXXDestructorDecl*>(Dtor));
@@ -4309,23 +4311,28 @@ ExprResult Sema::MaybeBindToTemporary(Expr *E) {
   }
 
   // That should be enough to guarantee that this type is complete.
-  // If it has a trivial destructor, we can avoid the extra copy.
   CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
-  if (RD->isInvalidDecl() || RD->hasTrivialDestructor())
+  if (RD->isInvalidDecl() || RD->isDependentContext())
     return Owned(E);
-
   CXXDestructorDecl *Destructor = LookupDestructor(RD);
 
-  CXXTemporary *Temp = CXXTemporary::Create(Context, Destructor);
   if (Destructor) {
     MarkFunctionReferenced(E->getExprLoc(), Destructor);
     CheckDestructorAccess(E->getExprLoc(), Destructor,
                           PDiag(diag::err_access_dtor_temp)
                             << E->getType());
+    DiagnoseUseOfDecl(Destructor, E->getExprLoc());
+  }
 
+  // If destructor is trivial, we can avoid the extra copy.
+  if (Destructor->isTrivial())
+    return Owned(E);
+
+  if (Destructor)
     // We need a cleanup, but we don't need to remember the temporary.
     ExprNeedsCleanups = true;
-  }
+
+  CXXTemporary *Temp = CXXTemporary::Create(Context, Destructor);
   return Owned(CXXBindTemporaryExpr::Create(Context, Temp, E));
 }
 

clang/test/SemaCXX/defaulted-private-dtor.cpp

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -verify -std=c++11 %s -fcxx-exceptions
+
+class BadDtor {
+  // A private, but nonetheless trivial, destructor.
+  ~BadDtor() = default; // expected-note 9{{here}}
+  friend class K;
+};
+void f() {
+  BadDtor *p = new BadDtor[3]; // expected-error {{private destructor}}
+  delete [] p; // expected-error {{private destructor}}
+  const BadDtor &dd2 = BadDtor(); // expected-error {{private destructor}}
+  BadDtor dd; // expected-error {{private destructor}}
+  throw dd; // expected-error {{private destructor}}
+}
+struct X : BadDtor { // expected-error {{private destructor}}
+  ~X() {}
+};
+struct Y {
+  BadDtor dd; // expected-error {{private destructor}}
+  ~Y() {}
+};
+struct Z : virtual BadDtor { // expected-error {{private destructor}}
+  ~Z() {}
+};
+BadDtor dd; // expected-error {{private destructor}}
+
+class K : BadDtor {
+  void f() {
+    BadDtor *p = new BadDtor[3];
+    delete [] p;
+    const BadDtor &dd2 = BadDtor();
+    BadDtor dd;
+    throw dd;
+
+    {
+      BadDtor x;
+      goto dont_call_dtor;
+    }
+dont_call_dtor:
+    ;
+  }
+  struct Z : virtual BadDtor {
+    ~Z() {}
+  };
+  BadDtor dd;
+  ~K();
+};

clang/test/SemaCXX/deleted-function.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -fcxx-exceptions %s
 
 int i = delete; // expected-error {{only functions can have deleted definitions}}
 
@@ -33,3 +33,25 @@ void test() {
   d->fn(); // expected-error {{attempt to use a deleted function}}
   int i = *d; // expected-error {{invokes a deleted function}}
 }
+
+struct DelDtor {
+  ~DelDtor() = delete; // expected-note 9{{here}}
+};
+void f() {
+  DelDtor *p = new DelDtor[3]; // expected-error {{attempt to use a deleted function}}
+  delete [] p; // expected-error {{attempt to use a deleted function}}
+  const DelDtor &dd2 = DelDtor(); // expected-error {{attempt to use a deleted function}}
+  DelDtor dd; // expected-error {{attempt to use a deleted function}}
+  throw dd; // expected-error {{attempt to use a deleted function}}
+}
+struct X : DelDtor {
+  ~X() {} // expected-error {{attempt to use a deleted function}}
+};
+struct Y {
+  DelDtor dd;
+  ~Y() {} // expected-error {{attempt to use a deleted function}}
+};
+struct Z : virtual DelDtor {
+  ~Z() {} // expected-error {{attempt to use a deleted function}}
+};
+DelDtor dd; // expected-error {{attempt to use a deleted function}}