Fix clang plugin to catch defaulted inlined ctors/dtors.

BUG=459607,436357

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

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

3 files changed, 56 insertions, 4 deletions

diff --git a/tools/clang/plugins/FindBadConstructsConsumer.cpp b/tools/clang/plugins/FindBadConstructsConsumer.cpp
index 92396e6..fe868c0 100644
--- a/tools/clang/plugins/FindBadConstructsConsumer.cpp
+++ b/tools/clang/plugins/FindBadConstructsConsumer.cpp
@@ -283,6 +283,9 @@ void FindBadConstructsConsumer::CheckCtorDtorWeight(
       for (CXXRecordDecl::ctor_iterator it = record->ctor_begin();
            it != record->ctor_end();
            ++it) {
+        // The current check is buggy. An implicit copy constructor does not
+        // have an inline body, so this check never fires for classes with a
+        // user-declared out-of-line constructor.
         if (it->hasInlineBody()) {
           if (it->isCopyConstructor() &&
               !record->hasUserDeclaredCopyConstructor()) {
@@ -293,6 +296,15 @@ void FindBadConstructsConsumer::CheckCtorDtorWeight(
             emitWarning(it->getInnerLocStart(),
                         "Complex constructor has an inlined body.");
           }
+        } else if (it->isInlined() && (!it->isCopyOrMoveConstructor() ||
+                                       it->isExplicitlyDefaulted())) {
+          // isInlined() is a more reliable check than hasInlineBody(), but
+          // unfortunately, it results in warnings for implicit copy/move
+          // constructors in the previously mentioned situation. To preserve
+          // compatibility with existing Chromium code, only warn if it's an
+          // explicitly defaulted copy or move constructor.
+          emitWarning(it->getInnerLocStart(),
+                      "Complex constructor has an inlined body.");
         }
       }
     }
@@ -306,7 +318,7 @@ void FindBadConstructsConsumer::CheckCtorDtorWeight(
                   "Complex class/struct needs an explicit out-of-line "
                   "destructor.");
     } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
-      if (dtor->hasInlineBody()) {
+      if (dtor->isInlined()) {
         emitWarning(dtor->getInnerLocStart(),
                     "Complex destructor has an inline body.");
       }
diff --git a/tools/clang/plugins/tests/missing_ctor.h b/tools/clang/plugins/tests/missing_ctor.h
index 1050457..0551fd7 100644
--- a/tools/clang/plugins/tests/missing_ctor.h
+++ b/tools/clang/plugins/tests/missing_ctor.h
@@ -8,6 +8,8 @@
 #include <string>
 #include <vector>
 
+// Note: this should warn for an implicit copy constructor too, but currently
+// doesn't, due to a plugin bug.
 class MissingCtorsArentOKInHeader {
  public:
 
@@ -16,4 +18,33 @@ class MissingCtorsArentOKInHeader {
   std::vector<std::string> 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 InlineImplicitMoveCtorOK {
+ public:
+  InlineImplicitMoveCtorOK();
+
+ private:
+  // ctor weight = 12, dtor weight = 9.
+  std::string one_;
+  std::string two_;
+  std::string three_;
+  int four_;
+  int five_;
+  int six_;
+};
+
+class ExplicitlyDefaultedInlineAlsoWarns {
+ public:
+  ExplicitlyDefaultedInlineAlsoWarns() = default;
+  ~ExplicitlyDefaultedInlineAlsoWarns() = default;
+  ExplicitlyDefaultedInlineAlsoWarns(
+      const ExplicitlyDefaultedInlineAlsoWarns&) = default;
+
+ private:
+  std::vector<int> one_;
+  std::vector<std::string> two_;
+
+};
+
 #endif  // MISSING_CTOR_H_
diff --git a/tools/clang/plugins/tests/missing_ctor.txt b/tools/clang/plugins/tests/missing_ctor.txt
index 301449c..0bc5696 100644
--- a/tools/clang/plugins/tests/missing_ctor.txt
+++ b/tools/clang/plugins/tests/missing_ctor.txt
@@ -1,6 +1,15 @@
 In file included from missing_ctor.cpp:5:
-./missing_ctor.h:11:1: warning: [chromium-style] Complex class/struct needs an explicit out-of-line constructor.
+./missing_ctor.h:13:1: warning: [chromium-style] Complex class/struct needs an explicit out-of-line constructor.
 class MissingCtorsArentOKInHeader {
 ^
-./missing_ctor.h:11:1: warning: [chromium-style] Complex class/struct needs an explicit out-of-line destructor.
-2 warnings generated.
+./missing_ctor.h:13:1: warning: [chromium-style] Complex class/struct needs an explicit out-of-line destructor.
+./missing_ctor.h:39:3: warning: [chromium-style] Complex constructor has an inlined body.
+  ExplicitlyDefaultedInlineAlsoWarns() = default;
+  ^
+./missing_ctor.h:41:3: warning: [chromium-style] Complex constructor has an inlined body.
+  ExplicitlyDefaultedInlineAlsoWarns(
+  ^
+./missing_ctor.h:40:3: warning: [chromium-style] Complex destructor has an inline body.
+  ~ExplicitlyDefaultedInlineAlsoWarns() = default;
+  ^
+5 warnings generated.