Check for public dtors on base::RefCounted types

tools/clang/plugins/FindBadConstructs.cpp

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This file defines a bunch of recurring problems in the Chromium C++ code.
 //
 // Checks that are implemented:
 // - Constructors/Destructors should not be inlined if they are of a complex
 //   class type.
 // - Missing "virtual" keywords on methods that should be virtual.
 // - Non-annotated overriding virtual methods.
 // - Virtual methods with nonempty implementations in their headers.
-//
-// Things that are still TODO:
-// - Deriving from base::RefCounted and friends should mandate non-public
-//   destructors.
+// - Classes that derive from base::RefCounted / base::RefCountedThreadSafe
+//   should have protected or private destructors.
 
 #include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/AST.h"
+#include "clang/AST/CXXInheritance.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include "ChromeClassTester.h"
 
 using namespace clang;
 
 namespace {
 
 bool TypeHasNonTrivialDtor(const Type* type) {
   if (const CXXRecordDecl* cxx_r = type->getCXXRecordDeclForPointerType())
     return cxx_r->hasTrivialDestructor();
 
   return false;
 }
 
 // Searches for constructs that we know we don't want in the Chromium code base.
 class FindBadConstructsConsumer : public ChromeClassTester {
  public:
   FindBadConstructsConsumer(CompilerInstance& instance)
       : ChromeClassTester(instance) {}
 
   virtual void CheckChromeClass(const SourceLocation& record_location,
                                 CXXRecordDecl* record) {
-    CheckCtorDtorWeight(record_location, record);
-    CheckVirtualMethods(record_location, record);
+    bool implementation_file = InImplementationFile(record_location);
+
+    if (!implementation_file) {
+      // Only check for "heavy" constructors/destructors in header files;
+      // within implementation files, there is no performance cost.
+      CheckCtorDtorWeight(record_location, record);
+
+      // Check that all virtual methods are marked accordingly with both
+      // virtual and OVERRIDE.
+      CheckVirtualMethods(record_location, record);
+    }
+
+    CheckRefCountedDtors(record_location, record);
+  }
+
+  // Returns true if |base| specifies one of the Chromium reference counted
+  // classes (base::RefCounted / base::RefCountedThreadSafe). |user_data| is
+  // ignored.
+  static bool IsRefCountedCallback(const CXXBaseSpecifier* base,
+                                   CXXBasePath& path,
+                                   void* user_data) {
+    FindBadConstructsConsumer* self =
+        static_cast<FindBadConstructsConsumer*>(user_data);
+    if (base->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() !=
+        TypeLoc::TemplateSpecialization) {
+      return false;
+    }
+
+    const TemplateSpecializationType* base_type =
+        dyn_cast<TemplateSpecializationType>(base->getType());
+    TemplateName name = base_type->getTemplateName();
+
+    if (TemplateDecl* decl = name.getAsTemplateDecl()) {
+      std::string base_name = decl->getNameAsString();
+      if (base_name.compare(0, 10, "RefCounted") == 0 &&
+          self->GetNamespace(decl) == "base") {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  // Prints errors if the destructor of a RefCounted class is public.
+  void CheckRefCountedDtors(const SourceLocation& record_location,
+                            CXXRecordDecl* record) {
+    // Skip anonymous structs.
+    if (record->getIdentifier() == NULL)
+      return;
+
+    CXXBasePaths paths;
+    if (!record->lookupInBases(
+            &FindBadConstructsConsumer::IsRefCountedCallback, this, paths)) {
+      return;  // Class does not derive from a ref-counted base class.
+    }
+
+    if (!record->hasUserDeclaredDestructor()) {
+      emitWarning(
+          record_location,
+          "Classes that are ref-counted should have explicit "
+          "destructors that are protected or private.");
+    } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
+      if (dtor->getAccess() == AS_public) {
+        emitWarning(
+            dtor->getInnerLocStart(),
+            "Classes that are ref-counted should not have "
+            "public destructors.");
+      }
+    }
   }
 
   // Prints errors if the constructor/destructor weight is too heavy.
   void CheckCtorDtorWeight(const SourceLocation& record_location,
                            CXXRecordDecl* record) {
     // We don't handle anonymous structs. If this record doesn't have a
     // name, it's of the form:
     //
     // struct {
     //   ...
@@ skipping 95 matching lines @@
         if (cs->size()) {
           emitWarning(
               cs->getLBracLoc(),
               "virtual methods with non-empty bodies shouldn't be "
               "declared inline.");
         }
       }
     }
   }
 
+  bool InTestingNamespace(const Decl* record) {
+    return GetNamespace(record).find("testing") != std::string::npos;
+  }
+
   bool IsMethodInBannedNamespace(const CXXMethodDecl* method) {
     if (InBannedNamespace(method))
       return true;
     for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods();
          i != method->end_overridden_methods();
          ++i) {
       const CXXMethodDecl* overridden = *i;
       if (IsMethodInBannedNamespace(overridden))
         return true;
     }
@@ skipping 118 matching lines @@
                  const std::vector<std::string>& args) {
     // We don't take any additional arguments here.
     return true;
   }
 };
 
 }  // namespace
 
 static FrontendPluginRegistry::Add<FindBadConstructsAction>
 X("find-bad-constructs", "Finds bad C++ constructs");