clang-plugin: Skip classes deriving from IPC::NoParams.

This patch changes the check for complex implicit constructors to skip
classes that derive (directly or indirectly) from IPC::NoParams.

This avoids us having to modify IPC macros to sometimes include
copy ctors.

R=dcheng@chromium.org, thakis@chromium.org
BUG=436357

Review URL: https://codereview.chromium.org/1743833003

Cr-Commit-Position: refs/heads/master@{#378250}

6 files changed, 94 insertions, 0 deletions

diff --git a/tools/clang/plugins/ChromeClassTester.cpp b/tools/clang/plugins/ChromeClassTester.cpp
index 5383a91..ad7b82b 100644
--- a/tools/clang/plugins/ChromeClassTester.cpp
+++ b/tools/clang/plugins/ChromeClassTester.cpp
@@ -175,6 +175,21 @@ std::string ChromeClassTester::GetNamespace(const Decl* record) {
   return GetNamespaceImpl(record->getDeclContext(), "");
 }
 
+bool ChromeClassTester::HasIgnoredBases(const CXXRecordDecl* record) {
+  for (const auto& base : record->bases()) {
+    CXXRecordDecl* base_record = base.getType()->getAsCXXRecordDecl();
+    if (!base_record)
+      continue;
+
+    const std::string& base_name = base_record->getQualifiedNameAsString();
+    if (ignored_base_classes_.count(base_name) > 0)
+      return true;
+    if (HasIgnoredBases(base_record))
+      return true;
+  }
+  return false;
+}
+
 bool ChromeClassTester::InImplementationFile(SourceLocation record_location) {
   std::string filename;
 
@@ -273,6 +288,10 @@ void ChromeClassTester::BuildBannedLists() {
 
   // Enum type with _LAST members where _LAST doesn't mean last enum value.
   ignored_record_names_.emplace("ViewID");
+
+  // Ignore IPC::NoParams bases, since these structs are generated via
+  // macros and it makes it difficult to add explicit ctors.
+  ignored_base_classes_.emplace("IPC::NoParams");
 }
 
 std::string ChromeClassTester::GetNamespaceImpl(const DeclContext* context,
diff --git a/tools/clang/plugins/ChromeClassTester.h b/tools/clang/plugins/ChromeClassTester.h
index 2dcfb42..22af158 100644
--- a/tools/clang/plugins/ChromeClassTester.h
+++ b/tools/clang/plugins/ChromeClassTester.h
@@ -47,6 +47,10 @@ class ChromeClassTester {
   // "<anonymous namespace>".
   std::string GetNamespace(const clang::Decl* record);
 
+  // Utility method to check whether the given record has any of the ignored
+  // base classes.
+  bool HasIgnoredBases(const clang::CXXRecordDecl* record);
+
   // Utility method for subclasses to check if this class is within an
   // implementation (.cc, .cpp, .mm) file.
   bool InImplementationFile(clang::SourceLocation location);
@@ -93,6 +97,9 @@ class ChromeClassTester {
 
   // List of types that we don't check.
   std::set<std::string> ignored_record_names_;
+
+  // List of base classes that we skip when checking complex class ctors/dtors.
+  std::set<std::string> ignored_base_classes_;
 };
 
 #endif  // TOOLS_CLANG_PLUGINS_CHROMECLASSTESTER_H_
diff --git a/tools/clang/plugins/FindBadConstructsConsumer.cpp b/tools/clang/plugins/FindBadConstructsConsumer.cpp
index b7e1886..d1fc29d 100644
--- a/tools/clang/plugins/FindBadConstructsConsumer.cpp
+++ b/tools/clang/plugins/FindBadConstructsConsumer.cpp
@@ -230,6 +230,10 @@ void FindBadConstructsConsumer::CheckCtorDtorWeight(
   if (record->getIdentifier() == NULL)
     return;
 
+  // Skip records that derive from ignored base classes.
+  if (HasIgnoredBases(record))
+    return;
+
   // Count the number of templated base classes as a feature of whether the
   // destructor can be inlined.
   int templated_base_classes = 0;
diff --git a/tools/clang/plugins/tests/missing_ctor_ignored_base.cpp b/tools/clang/plugins/tests/missing_ctor_ignored_base.cpp
new file mode 100644
index 0000000..6c4ddd4
--- /dev/null
+++ b/tools/clang/plugins/tests/missing_ctor_ignored_base.cpp
@@ -0,0 +1,11 @@
+// Copyright (c) 2016 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.
+
+#include "missing_ctor_ignored_base.h"
+
+int main() {
+  MissingCtorsWithIgnoredBase one;
+  MissingCtorsWithIgnoredGrandBase two;
+  return 0;
+}
diff --git a/tools/clang/plugins/tests/missing_ctor_ignored_base.h b/tools/clang/plugins/tests/missing_ctor_ignored_base.h
new file mode 100644
index 0000000..7c80e48
--- /dev/null
+++ b/tools/clang/plugins/tests/missing_ctor_ignored_base.h
@@ -0,0 +1,53 @@
+// Copyright (c) 2016 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.
+
+#ifndef MISSING_CTOR_IGNORED_BASE_H_
+#define MISSING_CTOR_IGNORED_BASE_H_
+
+struct MyString {
+  MyString();
+  ~MyString();
+  MyString(const MyString&);
+  MyString(MyString&&);
+};
+
+template <class T>
+struct MyVector {
+  MyVector();
+  ~MyVector();
+  MyVector(const MyVector&);
+  MyVector(MyVector&&);
+};
+
+// IPC::NoParams is an ignored base.
+namespace IPC {
+class NoParams {};
+}
+
+// Note: this should warn for an implicit copy constructor too, but currently
+// doesn't, due to a plugin bug.
+class MissingCtorsWithIgnoredBase : public IPC::NoParams {
+ public:
+
+ private:
+  MyVector<int> one_;
+  MyVector<MyString> two_;
+};
+
+// Inline move ctors shouldn't be warned about. Similar to the previous test
+// case, this also incorrectly fails to warn for the implicit copy ctor.
+class MissingCtorsWithIgnoredGrandBase : public MissingCtorsWithIgnoredBase {
+ public:
+
+ private:
+  // ctor weight = 12, dtor weight = 9.
+  MyString one_;
+  MyString two_;
+  MyString three_;
+  int four_;
+  int five_;
+  int six_;
+};
+
+#endif  // MISSING_CTOR_IGNORED_BASE_H_
diff --git a/tools/clang/plugins/tests/missing_ctor_ignored_base.txt b/tools/clang/plugins/tests/missing_ctor_ignored_base.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tools/clang/plugins/tests/missing_ctor_ignored_base.txt