Check for public dtors on base::RefCounted types

tools/clang/plugins/ChromeClassTester.cpp

Index: tools/clang/plugins/ChromeClassTester.cpp
diff --git a/tools/clang/plugins/ChromeClassTester.cpp b/tools/clang/plugins/ChromeClassTester.cpp
index dd983bd81410cb1460ec27747ccfa0a7fc2836a7..7a3b89ebc91c84d0093aa4317464609d9194b248 100644
--- a/tools/clang/plugins/ChromeClassTester.cpp
+++ b/tools/clang/plugins/ChromeClassTester.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -40,72 +40,6 @@ ChromeClassTester::ChromeClassTester(CompilerInstance& instance)
   BuildBannedLists();
 }
 
-void ChromeClassTester::BuildBannedLists() {
-  banned_namespaces_.push_back("std");
-  banned_namespaces_.push_back("__gnu_cxx");
-  banned_namespaces_.push_back("WebKit");
-
-  banned_directories_.push_back("third_party/");
-  banned_directories_.push_back("native_client/");
-  banned_directories_.push_back("breakpad/");
-  banned_directories_.push_back("courgette/");
-  banned_directories_.push_back("pdf/");
-  banned_directories_.push_back("ppapi/");
-  banned_directories_.push_back("usr/");
-  banned_directories_.push_back("testing/");
-  banned_directories_.push_back("googleurl/");
-  banned_directories_.push_back("v8/");
-  banned_directories_.push_back("dart/");
-  banned_directories_.push_back("sdch/");
-  banned_directories_.push_back("icu4c/");
-  banned_directories_.push_back("frameworks/");
-
-  // Don't check autogenerated headers.
-  // Make puts them below $(builddir_name)/.../gen and geni.
-  // Ninja puts them below OUTPUT_DIR/.../gen
-  // Xcode has a fixed output directory for everything.
-  banned_directories_.push_back("gen/");
-  banned_directories_.push_back("geni/");
-  banned_directories_.push_back("xcodebuild/");
-
-  // You are standing in a mazy of twisty dependencies, all resolved by
-  // putting everything in the header.
-  banned_directories_.push_back("automation/");
-
-  // Don't check system headers.
-  banned_directories_.push_back("/Developer/");
-
-  // Used in really low level threading code that probably shouldn't be out of
-  // lined.
-  ignored_record_names_.insert("ThreadLocalBoolean");
-
-  // A complicated pickle derived struct that is all packed integers.
-  ignored_record_names_.insert("Header");
-
-  // Part of the GPU system that uses multiple included header
-  // weirdness. Never getting this right.
-  ignored_record_names_.insert("Validators");
-
-  // Has a UNIT_TEST only constructor. Isn't *terribly* complex...
-  ignored_record_names_.insert("AutocompleteController");
-  ignored_record_names_.insert("HistoryURLProvider");
-
-  // Because of chrome frame
-  ignored_record_names_.insert("ReliabilityTestSuite");
-
-  // Used over in the net unittests. A large enough bundle of integers with 1
-  // non-pod class member. Probably harmless.
-  ignored_record_names_.insert("MockTransaction");
-
-  // Used heavily in ui_unittests and once in views_unittests. Fixing this
-  // isn't worth the overhead of an additional library.
-  ignored_record_names_.insert("TestAnimationDelegate");
-
-  // Part of our public interface that nacl and friends use. (Arguably, this
-  // should mean that this is a higher priority but fixing this looks hard.)
-  ignored_record_names_.insert("PluginVersionInfo");
-}
-
 ChromeClassTester::~ChromeClassTester() {}
 
 void ChromeClassTester::HandleTagDeclDefinition(TagDecl* tag) {
@@ -146,7 +80,8 @@ void ChromeClassTester::HandleTagDeclDefinition(TagDecl* tag) {
   }
 }
 
-void ChromeClassTester::emitWarning(SourceLocation loc, const char* raw_error) {
+void ChromeClassTester::emitWarning(SourceLocation loc,
+                                    const char* raw_error) {
   FullSourceLoc full(loc, instance().getSourceManager());
   std::string err;
   err = "[chromium-style] ";
@@ -159,10 +94,6 @@ void ChromeClassTester::emitWarning(SourceLocation loc, const char* raw_error) {
   DiagnosticBuilder B = diagnostic().Report(full, id);
 }
 
-bool ChromeClassTester::InTestingNamespace(const Decl* record) {
-  return GetNamespace(record).find("testing") != std::string::npos;
-}
-
 bool ChromeClassTester::InBannedNamespace(const Decl* record) {
   std::string n = GetNamespace(record);
   if (n != "") {
@@ -177,8 +108,88 @@ std::string ChromeClassTester::GetNamespace(const Decl* record) {
   return GetNamespaceImpl(record->getDeclContext(), "");
 }
 
+bool ChromeClassTester::InImplementationFile(SourceLocation record_location) {
+  std::string filename;
+  if (!GetFilename(record_location, &filename)) {
+    return false;
+  }
+
+  if (ends_with(filename, ".cc") || ends_with(filename, ".cpp") ||
+      ends_with(filename, ".mm")) {
+    return true;
+  }
+
+  return false;
+}
+
+void ChromeClassTester::BuildBannedLists() {
+  banned_namespaces_.push_back("std");
+  banned_namespaces_.push_back("__gnu_cxx");
+  banned_namespaces_.push_back("WebKit");
+
+  banned_directories_.push_back("third_party/");
+  banned_directories_.push_back("native_client/");
+  banned_directories_.push_back("breakpad/");
+  banned_directories_.push_back("courgette/");
+  banned_directories_.push_back("pdf/");
+  banned_directories_.push_back("ppapi/");
+  banned_directories_.push_back("usr/");
+  banned_directories_.push_back("testing/");
+  banned_directories_.push_back("googleurl/");
+  banned_directories_.push_back("v8/");
+  banned_directories_.push_back("dart/");
+  banned_directories_.push_back("sdch/");
+  banned_directories_.push_back("icu4c/");
+  banned_directories_.push_back("frameworks/");
+
+  // Don't check autogenerated headers.
+  // Make puts them below $(builddir_name)/.../gen and geni.
+  // Ninja puts them below OUTPUT_DIR/.../gen
+  // Xcode has a fixed output directory for everything.
+  banned_directories_.push_back("gen/");
+  banned_directories_.push_back("geni/");
+  banned_directories_.push_back("xcodebuild/");
+
+  // You are standing in a mazy of twisty dependencies, all resolved by
+  // putting everything in the header.
+  banned_directories_.push_back("automation/");
+
+  // Don't check system headers.
+  banned_directories_.push_back("/Developer/");
+
+  // Used in really low level threading code that probably shouldn't be out of
+  // lined.
+  ignored_record_names_.insert("ThreadLocalBoolean");
+
+  // A complicated pickle derived struct that is all packed integers.
+  ignored_record_names_.insert("Header");
+
+  // Part of the GPU system that uses multiple included header
+  // weirdness. Never getting this right.
+  ignored_record_names_.insert("Validators");
+
+  // Has a UNIT_TEST only constructor. Isn't *terribly* complex...
+  ignored_record_names_.insert("AutocompleteController");
+  ignored_record_names_.insert("HistoryURLProvider");
+
+  // Because of chrome frame
+  ignored_record_names_.insert("ReliabilityTestSuite");
+
+  // Used over in the net unittests. A large enough bundle of integers with 1
+  // non-pod class member. Probably harmless.
+  ignored_record_names_.insert("MockTransaction");
+
+  // Used heavily in ui_unittests and once in views_unittests. Fixing this
+  // isn't worth the overhead of an additional library.
+  ignored_record_names_.insert("TestAnimationDelegate");
+
+  // Part of our public interface that nacl and friends use. (Arguably, this
+  // should mean that this is a higher priority but fixing this looks hard.)
+  ignored_record_names_.insert("PluginVersionInfo");
+}
+
 std::string ChromeClassTester::GetNamespaceImpl(const DeclContext* context,
-                                                std::string candidate) {
+                                                const std::string& candidate) {
   switch (context->getDeclKind()) {
     case Decl::TranslationUnit: {
       return candidate;
@@ -201,60 +212,49 @@ std::string ChromeClassTester::GetNamespaceImpl(const DeclContext* context,
 }
 
 bool ChromeClassTester::InBannedDirectory(SourceLocation loc) {
-  const SourceManager &SM = instance_.getSourceManager();
-  SourceLocation spelling_location = SM.getSpellingLoc(loc);
-  PresumedLoc ploc = SM.getPresumedLoc(spelling_location);
-  std::string buffer_name;
-  if (ploc.isInvalid()) {
-    // If we're in an invalid location, we're looking at things that aren't
-    // actually stated in the source; treat this as a banned location instead
-    // of going through our full lookup process.
+  std::string filename;
+  if (!GetFilename(loc, &filename)) {
+    // If the filename cannot be determined, simply treat this as a banned
+    // location, instead of going through the full lookup process.
     return true;
-  } else {
-    std::string b = ploc.getFilename();
-
-    // We need to special case scratch space; which is where clang does its
-    // macro expansion. We explicitly want to allow people to do otherwise bad
-    // things through macros that were defined due to third party libraries.
-    if (b == "<scratch space>")
-      return true;
-
-    // Don't complain about these things in implementation files.
-    if (ends_with(b, ".cc") || ends_with(b, ".cpp") || ends_with(b, ".mm")) {
-      return true;
-    }
+  }
 
-    // Don't complain about autogenerated protobuf files.
-    if (ends_with(b, ".pb.h")) {
-      return true;
-    }
+  // We need to special case scratch space; which is where clang does its
+  // macro expansion. We explicitly want to allow people to do otherwise bad
+  // things through macros that were defined due to third party libraries.
+  if (filename == "<scratch space>")
+    return true;
 
-    // We need to munge the paths so that they are relative to the repository
-    // srcroot. We first resolve the symlinktastic relative path and then
-    // remove our known srcroot from it if needed.
-    char resolvedPath[MAXPATHLEN];
-    if (realpath(b.c_str(), resolvedPath)) {
-      b = resolvedPath;
-    }
+  // Don't complain about autogenerated protobuf files.
+  if (ends_with(filename, ".pb.h")) {
+    return true;
+  }
+
+  // We need to munge the paths so that they are relative to the repository
+  // srcroot. We first resolve the symlinktastic relative path and then
+  // remove our known srcroot from it if needed.
+  char resolvedPath[MAXPATHLEN];
+  if (realpath(filename.c_str(), resolvedPath)) {
+    filename = resolvedPath;
+  }
 
-    // On linux, chrome is often checked out to /usr/local/google. Due to the
-    // "usr" rule in banned_directories_, all diagnostics would be suppressed
-    // in that case. As a workaround, strip that prefix.
-    b = lstrip(b, "/usr/local/google");
-
-    for (std::vector<std::string>::const_iterator it =
-             banned_directories_.begin();
-         it != banned_directories_.end(); ++it) {
-      // If we can find any of the banned path components in this path, then
-      // this file is rejected.
-      size_t index = b.find(*it);
-      if (index != std::string::npos) {
-        bool matches_full_dir_name = index == 0 || b[index - 1] == '/';
-        if ((*it)[0] == '/')
-          matches_full_dir_name = true;
-        if (matches_full_dir_name)
-          return true;
-      }
+  // On linux, chrome is often checked out to /usr/local/google. Due to the
+  // "usr" rule in banned_directories_, all diagnostics would be suppressed
+  // in that case. As a workaround, strip that prefix.
+  filename = lstrip(filename, "/usr/local/google");
+
+  for (std::vector<std::string>::const_iterator it =
+           banned_directories_.begin();
+       it != banned_directories_.end(); ++it) {
+    // If we can find any of the banned path components in this path, then
+    // this file is rejected.
+    size_t index = filename.find(*it);
+    if (index != std::string::npos) {
+      bool matches_full_dir_name = index == 0 || filename[index - 1] == '/';
+      if ((*it)[0] == '/')
+        matches_full_dir_name = true;
+      if (matches_full_dir_name)
+        return true;
     }
   }
 
@@ -264,3 +264,18 @@ bool ChromeClassTester::InBannedDirectory(SourceLocation loc) {
 bool ChromeClassTester::IsIgnoredType(const std::string& base_name) {
   return ignored_record_names_.find(base_name) != ignored_record_names_.end();
 }
+
+bool ChromeClassTester::GetFilename(SourceLocation loc,
+                                    std::string* filename) {
+  const SourceManager &SM = instance_.getSourceManager();
+  SourceLocation spelling_location = SM.getSpellingLoc(loc);
+  PresumedLoc ploc = SM.getPresumedLoc(spelling_location);
+  if (ploc.isInvalid()) {
+    // If we're in an invalid location, we're looking at things that aren't
+    // actually stated in the source.
+    return false;
+  }
+
+  *filename = ploc.getFilename();
+  return true;
+}