Fix PR 33189: Clang assertion on template destructor declaration

[kuang_he]

Summary

This patch aims to fix the bug reported at https://bugs.llvm.org/show_bug.cgi?id=33189. Clang hits an assertion when a template destructor declaration is present. This is caused by later processing that does not expect to encounter a template when looking at a destructor. The resolution is to treat the destructor as being not declared when later processing is interested in the properties of the destructor of a class.

[hubert.reinterpretcast]

lib/AST/DeclCXX.cpp

1419–1420

As it is,

return R.empty() ? nullptr : dyn_cast<CXXDestructorDecl>(R.front());

would probably be much more succinct.

[kuang_he]

Patch updated addressing comment.

[kuang_he]

Can we have this patch reviewed?

[kuang_he]

Can we get this patch reviewed by any chance?

[hubert.reinterpretcast]

In D33833#789436, @kuang_he wrote:

Can we get this patch reviewed by any chance?

@kuang_he; it is customary to "ping". In this case, "Ping 2".

lib/AST/DeclCXX.cpp

1421

I think what is here is probably what we want to do. My understanding is that the code is certainly invalid before we hit this, and the case is so odd that pursuing better recovery is not worthwhile.

Do we know if we can recover from getting a FunctionTemplateDecl by some other means? Perhaps by using the result of getTemplatedDecl()? I suspect there may be problems with cases where the noexcept-specifier is dependent.

[rsmith]

test/SemaCXX/PR33189.cpp

1–7

Please fold this into an existing test file, perhaps test/SemaTemplate/destructor-template.cpp?

[rsmith]

Thanks, do you need someone to commit this for you?

lib/AST/DeclCXX.cpp

 CXXDestructorDecl *CXXRecordDecl::getDestructor() const {
   ASTContext &Context = getASTContext();
   QualType ClassType = Context.getTypeDeclType(this);
 
   DeclarationName Name
     = Context.DeclarationNames.getCXXDestructorName(
                                           Context.getCanonicalType(ClassType));
 
   DeclContext::lookup_result R = lookup(Name);
-  if (R.empty())
-    return nullptr;
 
-  CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(R.front());
-  return Dtor;
+  return R.empty() ? nullptr : dyn_cast<CXXDestructorDecl>(R.front());
 }
 
 bool CXXRecordDecl::isAnyDestructorNoReturn() const {
   // Destructor is noreturn.
   if (const CXXDestructorDecl *Destructor = getDestructor())
     if (Destructor->isNoReturn())
       return true;
 

test/SemaCXX/PR33189.cpp

+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+
+class U
+{
+  template <class T>
+  ~U() { } // expected-error{{destructor cannot be declared as a template}}
+};