[clang][Sema] Fix crash on invalid base destructor

[kadircet]

Summary

LookupSpecialMember might fail, so changes the cast to cast_or_null.
Inside Sema, skip a particular base, similar to other cases, rather than
asserting on dtor showing up.

Other option would be to mark classes with invalid destructors as invalid, but
that seems like a lot more invasive and we do lose lots of diagnostics that
currently work on classes with broken members.

[hokein]

looks good, Sema::LookupDefaultConstructor already does the similar thing as well.

clang/lib/Sema/SemaDeclCXX.cpp

5702–5704

I think we should do similar change on L5699, L5736 as well.

clang/lib/Sema/SemaLookup.cpp

3620–3623

nit: the prefix llvm:: can be eliminated.

[kadircet]

-address comments

clang/lib/Sema/SemaDeclCXX.cpp

 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ASTMutationListener.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/ComparisonCategories.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeOrdering.h"
if (FieldClassDecl->isInvalidDecl())
       continue;
     if (FieldClassDecl->hasIrrelevantDestructor())
       continue;
     // The destructor for an implicit anonymous union member is never invoked.
     if (FieldClassDecl->isUnion() && FieldClassDecl->isAnonymousStructOrUnion())
       continue;
 
     CXXDestructorDecl *Dtor = LookupDestructor(FieldClassDecl);
-    assert(Dtor && "No dtor found for FieldClassDecl!");
+    // Dtor might still be missing, e.g because it's invalid.
+    if (!Dtor)
+      continue;
     CheckDestructorAccess(Field->getLocation(), Dtor,
                           PDiag(diag::err_access_dtor_field)
                             << Field->getDeclName()
                             << FieldType);
 
     MarkFunctionReferenced(Location, Dtor);
     DiagnoseUseOfDecl(Dtor, Location);
   }
for (const auto &Base : ClassDecl->bases()) {
     CXXRecordDecl *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
     // If our base class is invalid, we probably can't get its dtor anyway.
     if (BaseClassDecl->isInvalidDecl())
       continue;
     if (BaseClassDecl->hasIrrelevantDestructor())
       continue;
 
     CXXDestructorDecl *Dtor = LookupDestructor(BaseClassDecl);
-    assert(Dtor && "No dtor found for BaseClassDecl!");
+    // Dtor might still be missing, e.g because it's invalid.
+    if (!Dtor)
+      continue;
 
     // FIXME: caret should be on the start of the class name
     CheckDestructorAccess(Base.getBeginLoc(), Dtor,
                           PDiag(diag::err_access_dtor_base)
                               << Base.getType() << Base.getSourceRange(),
                           Context.getTypeDeclType(ClassDecl));
 
     MarkFunctionReferenced(Location, Dtor);
for (const auto &VBase : ClassDecl->vbases()) {
     CXXRecordDecl *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
     // If our base class is invalid, we probably can't get its dtor anyway.
     if (BaseClassDecl->isInvalidDecl())
       continue;
     if (BaseClassDecl->hasIrrelevantDestructor())
       continue;
 
     CXXDestructorDecl *Dtor = LookupDestructor(BaseClassDecl);
-    assert(Dtor && "No dtor found for BaseClassDecl!");
+    // Dtor might still be missing, e.g because it's invalid.
+    if (!Dtor)
+      continue;
     if (CheckDestructorAccess(
             ClassDecl->getLocation(), Dtor,
             PDiag(diag::err_access_dtor_vbase)
                 << Context.getTypeDeclType(ClassDecl) << VBase.getType(),
             Context.getTypeDeclType(ClassDecl)) ==
         AR_accessible) {
       CheckDerivedToBaseConversion(
           Context.getTypeDeclType(ClassDecl), VBase.getType(),

clang/lib/Sema/SemaLookup.cpp

 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/ADT/edit_distance.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <algorithm>
 #include <iterator>
 #include <list>
 #include <set>
 #include <utility>
 #include <vector>
 

 
 /// Look for the destructor of the given class.
 ///
 /// During semantic analysis, this routine should be used in lieu of
 /// CXXRecordDecl::getDestructor().
 ///
 /// \returns The destructor for this class.
 CXXDestructorDecl *Sema::LookupDestructor(CXXRecordDecl *Class) {
-  return cast<CXXDestructorDecl>(LookupSpecialMember(Class, CXXDestructor,
-                                                     false, false, false,
-                                                     false, false).getMethod());
+  return cast_or_null<CXXDestructorDecl>(
+      LookupSpecialMember(Class, CXXDestructor, false, false, false, false,
+                          false)
+          .getMethod());
 }
 
 /// LookupLiteralOperator - Determine which literal operator should be used for
 /// a user-defined literal, per C++11 [lex.ext].
 ///
 /// Normal overload resolution is not used to select which literal operator to
 /// call for a user-defined literal. Look up the provided literal operator name,
 /// and filter the results to the appropriate set for the given argument types.

clang/test/SemaCXX/destructor.cpp

namespace PR44978 {
   namespace { using U = Q; } // expected-note {{candidate}} expected-note {{found}}
   using U = int; // expected-note {{candidate}} expected-note {{found}}
   void f(Q *p) { p->~U(); } // expected-warning {{ambiguous}}
 
   // We still diagnose if the unqualified lookup is dependent, though.
   template<typename T> void f(T *p) { p->~U(); } // expected-error {{ambiguous}}
 }
 
+namespace crash_on_invalid_base_dtor {
+struct Test {
+  virtual ~Test();
+};
+struct Baz : public Test { // expected-warning {{non-virtual destructor}}
+  Baz() {}
+  ~Baz() = defaul; // expected-error {{undeclared identifier 'defaul'}} \
+                   // expected-error {{initializer on function}} \
+                   // expected-note {{overridden virtual function is here}}
+};
+struct Foo : public Baz { // expected-error {{cannot override a non-deleted function}} \
+                          // expected-note {{destructor of 'Foo' is implicitly deleted}}
+  Foo() {}
+};
+}
+
 #endif // BE_THE_HEADER