[clang-tidy] ClangTidy check to flag uninitialized builtin and pointer fields.

Summary:
This patch is a continuation of http://reviews.llvm.org/D10553 by Jonathan B Coe.

The main additions are:

1. For C++11 the check suggests in-class field initialization as fix. This
makes the fields future proof towards the addition of new constructors.
2 For older language versions the fields are added in the right position
in the initializer list with more tests.
3. User documentation.

Reviewers: alexfh, jbcoe

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D16517

llvm-svn: 260873

4 files changed, 278 insertions, 0 deletions

diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
index d466ca83566..ea97ebe2dee 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -7,6 +7,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule
   ProBoundsPointerArithmeticCheck.cpp
   ProTypeConstCastCheck.cpp
   ProTypeCstyleCastCheck.cpp
+  ProTypeMemberInitCheck.cpp
   ProTypeReinterpretCastCheck.cpp
   ProTypeStaticCastDowncastCheck.cpp
   ProTypeUnionAccessCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
index db783a3975d..70fa0f493a5 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -16,6 +16,7 @@
 #include "ProBoundsPointerArithmeticCheck.h"
 #include "ProTypeConstCastCheck.h"
 #include "ProTypeCstyleCastCheck.h"
+#include "ProTypeMemberInitCheck.h"
 #include "ProTypeReinterpretCastCheck.h"
 #include "ProTypeStaticCastDowncastCheck.h"
 #include "ProTypeUnionAccessCheck.h"
@@ -39,6 +40,8 @@ public:
         "cppcoreguidelines-pro-type-const-cast");
     CheckFactories.registerCheck<ProTypeCstyleCastCheck>(
         "cppcoreguidelines-pro-type-cstyle-cast");
+    CheckFactories.registerCheck<ProTypeMemberInitCheck>(
+        "cppcoreguidelines-pro-type-member-init");
     CheckFactories.registerCheck<ProTypeReinterpretCastCheck>(
         "cppcoreguidelines-pro-type-reinterpret-cast");
     CheckFactories.registerCheck<ProTypeStaticCastDowncastCheck>(
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
new file mode 100644
index 00000000000..6e5896a9f49
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -0,0 +1,231 @@
+//===--- ProTypeMemberInitCheck.cpp - clang-tidy---------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ProTypeMemberInitCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/SmallPtrSet.h"
+
+using namespace clang::ast_matchers;
+using llvm::SmallPtrSet;
+using llvm::SmallPtrSetImpl;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+namespace {
+
+AST_MATCHER(CXXConstructorDecl, isUserProvided) {
+  return Node.isUserProvided();
+}
+
+static void
+fieldsRequiringInit(const RecordDecl::field_range &Fields,
+                    SmallPtrSetImpl<const FieldDecl *> &FieldsToInit) {
+  for (const FieldDecl *F : Fields) {
+    QualType Type = F->getType();
+    if (Type->isPointerType() || Type->isBuiltinType())
+      FieldsToInit.insert(F);
+  }
+}
+
+void removeFieldsInitializedInBody(
+    const Stmt &Stmt, ASTContext &Context,
+    SmallPtrSetImpl<const FieldDecl *> &FieldDecls) {
+  auto Matches =
+      match(findAll(binaryOperator(
+                hasOperatorName("="),
+                hasLHS(memberExpr(member(fieldDecl().bind("fieldDecl")))))),
+            Stmt, Context);
+  for (const auto &Match : Matches)
+    FieldDecls.erase(Match.getNodeAs<FieldDecl>("fieldDecl"));
+}
+
+// Creates comma separated list of fields requiring initialization in order of
+// declaration.
+std::string toCommaSeparatedString(
+    const RecordDecl::field_range &FieldRange,
+    const SmallPtrSetImpl<const FieldDecl *> &FieldsRequiringInit) {
+  std::string List;
+  llvm::raw_string_ostream Stream(List);
+  size_t AddedFields = 0;
+  for (const FieldDecl *Field : FieldRange) {
+    if (FieldsRequiringInit.count(Field) > 0) {
+      Stream << Field->getName();
+      if (++AddedFields < FieldsRequiringInit.size())
+        Stream << ", ";
+    }
+  }
+  return Stream.str();
+}
+
+// Contains all fields in correct order that need to be inserted at the same
+// location for pre C++11.
+// There are 3 kinds of insertions:
+// 1. The fields are inserted after an existing CXXCtorInitializer stored in
+// InitializerBefore. This will be the case whenever there is a written
+// initializer before the fields available.
+// 2. The fields are inserted before the first existing initializer stored in
+// InitializerAfter.
+// 3. There are no written initializers and the fields will be inserted before
+// the constructor's body creating a new initializer list including the ':'.
+struct FieldsInsertion {
+  const CXXCtorInitializer *InitializerBefore;
+  const CXXCtorInitializer *InitializerAfter;
+  SmallVector<const FieldDecl *, 4> Fields;
+
+  SourceLocation getLocation(const ASTContext &Context,
+                             const CXXConstructorDecl &Constructor) const {
+    if (InitializerBefore != nullptr) {
+      return Lexer::getLocForEndOfToken(InitializerBefore->getRParenLoc(), 0,
+                                        Context.getSourceManager(),
+                                        Context.getLangOpts());
+    }
+    auto StartLocation = InitializerAfter != nullptr
+                             ? InitializerAfter->getSourceRange().getBegin()
+                             : Constructor.getBody()->getLocStart();
+    auto Token =
+        lexer_utils::getPreviousNonCommentToken(Context, StartLocation);
+    return Lexer::getLocForEndOfToken(Token.getLocation(), 0,
+                                      Context.getSourceManager(),
+                                      Context.getLangOpts());
+  }
+
+  std::string codeToInsert() const {
+    assert(!Fields.empty() && "No fields to insert");
+    std::string Code;
+    llvm::raw_string_ostream Stream(Code);
+    // Code will be inserted before the first written initializer after ':',
+    // append commas.
+    if (InitializerAfter != nullptr) {
+      for (const auto *Field : Fields)
+        Stream << " " << Field->getName() << "(),";
+    } else {
+      // The full initializer list is created, add extra space after
+      // constructor's rparens.
+      if (InitializerBefore == nullptr)
+        Stream << " ";
+      for (const auto *Field : Fields)
+        Stream << ", " << Field->getName() << "()";
+    }
+    Stream.flush();
+    // The initializer list is created, replace leading comma with colon.
+    if (InitializerBefore == nullptr && InitializerAfter == nullptr)
+      Code[1] = ':';
+    return Code;
+  }
+};
+
+SmallVector<FieldsInsertion, 16> computeInsertions(
+    const CXXConstructorDecl::init_const_range &Inits,
+    const RecordDecl::field_range &Fields,
+    const SmallPtrSetImpl<const FieldDecl *> &FieldsRequiringInit) {
+  // Find last written non-member initializer or null.
+  const CXXCtorInitializer *LastWrittenNonMemberInit = nullptr;
+  for (const CXXCtorInitializer *Init : Inits) {
+    if (Init->isWritten() && !Init->isMemberInitializer())
+      LastWrittenNonMemberInit = Init;
+  }
+  SmallVector<FieldsInsertion, 16> OrderedFields;
+  OrderedFields.push_back({LastWrittenNonMemberInit, nullptr, {}});
+
+  auto CurrentField = Fields.begin();
+  for (const CXXCtorInitializer *Init : Inits) {
+    if (Init->isWritten() && Init->isMemberInitializer()) {
+      const FieldDecl *MemberField = Init->getMember();
+      // Add all fields between current field and this member field the previous
+      // FieldsInsertion if the field requires initialization.
+      for (; CurrentField != Fields.end() && *CurrentField != MemberField;
+           ++CurrentField) {
+        if (FieldsRequiringInit.count(*CurrentField) > 0)
+          OrderedFields.back().Fields.push_back(*CurrentField);
+      }
+      // If this is the first written member initializer and there was no
+      // written non-member initializer set this initializer as
+      // InitializerAfter.
+      if (OrderedFields.size() == 1 &&
+          OrderedFields.back().InitializerBefore == nullptr)
+        OrderedFields.back().InitializerAfter = Init;
+      OrderedFields.push_back({Init, nullptr, {}});
+    }
+  }
+  // Add remaining fields that require initialization to last FieldsInsertion.
+  for (; CurrentField != Fields.end(); ++CurrentField) {
+    if (FieldsRequiringInit.count(*CurrentField) > 0)
+      OrderedFields.back().Fields.push_back(*CurrentField);
+  }
+  return OrderedFields;
+}
+
+} // namespace
+
+void ProTypeMemberInitCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(cxxConstructorDecl(isDefinition(), isUserProvided(),
+                                        unless(isInstantiated()))
+                         .bind("ctor"),
+                     this);
+}
+
+void ProTypeMemberInitCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
+  const auto &MemberFields = Ctor->getParent()->fields();
+
+  SmallPtrSet<const FieldDecl *, 16> FieldsToInit;
+  fieldsRequiringInit(MemberFields, FieldsToInit);
+  if (FieldsToInit.empty())
+    return;
+
+  for (CXXCtorInitializer *Init : Ctor->inits()) {
+    // Return early if this constructor simply delegates to another constructor
+    // in the same class.
+    if (Init->isDelegatingInitializer())
+      return;
+    if (!Init->isMemberInitializer())
+      continue;
+    FieldsToInit.erase(Init->getMember());
+  }
+  removeFieldsInitializedInBody(*Ctor->getBody(), *Result.Context,
+                                FieldsToInit);
+  if (FieldsToInit.empty())
+    return;
+
+  DiagnosticBuilder Diag =
+      diag(Ctor->getLocStart(),
+           "constructor does not initialize these built-in/pointer fields: %0")
+      << toCommaSeparatedString(MemberFields, FieldsToInit);
+  // Do not propose fixes in macros since we cannot place them correctly.
+  if (Ctor->getLocStart().isMacroID())
+    return;
+  // For C+11 use in-class initialization which covers all future constructors
+  // as well.
+  if (Result.Context->getLangOpts().CPlusPlus11) {
+    for (const auto *Field : FieldsToInit) {
+      Diag << FixItHint::CreateInsertion(
+          Lexer::getLocForEndOfToken(Field->getSourceRange().getEnd(), 0,
+                                     Result.Context->getSourceManager(),
+                                     Result.Context->getLangOpts()),
+          "{}");
+    }
+    return;
+  }
+  for (const auto &FieldsInsertion :
+       computeInsertions(Ctor->inits(), MemberFields, FieldsToInit)) {
+    if (!FieldsInsertion.Fields.empty())
+      Diag << FixItHint::CreateInsertion(
+          FieldsInsertion.getLocation(*Result.Context, *Ctor),
+          FieldsInsertion.codeToInsert());
+  }
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
new file mode 100644
index 00000000000..60765b82750
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
@@ -0,0 +1,43 @@
+//===--- ProTypeMemberInitCheck.h - clang-tidy-------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// \brief Checks that builtin or pointer fields are initialized by
+/// user-defined constructors.
+///
+/// Members initialized through function calls in the body of the constructor
+/// will result in false positives.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.html
+/// TODO: See if 'fixes' for false positives are optimized away by the compiler.
+/// TODO: "Issue a diagnostic when constructing an object of a trivially
+/// constructible type without () or {} to initialize its members. To fix: Add
+/// () or {}."
+class ProTypeMemberInitCheck : public ClangTidyCheck {
+public:
+  ProTypeMemberInitCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H