flx removed rL LLVM as the repository for this revision.
flx updated this revision to Diff 45837.
flx added a comment.

Removed unused include and changed casing on Builder variable.


http://reviews.llvm.org/D16517

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/UninitializedFieldCheck.cpp
  clang-tidy/misc/UninitializedFieldCheck.h
  docs/clang-tidy/checks/misc-uninitialized-field.rst
  test/clang-tidy/misc-uninitialized-field-cxx98.cpp
  test/clang-tidy/misc-uninitialized-field.cpp

Index: test/clang-tidy/misc-uninitialized-field.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-uninitialized-field.cpp
@@ -0,0 +1,69 @@
+// RUN: %check_clang_tidy %s misc-uninitialized-field %t
+
+struct PositiveFieldBeforeConstructor {
+  int F;
+  // CHECK-FIXES: int F{};
+  PositiveFieldBeforeConstructor() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F
+};
+
+struct PositiveFieldAfterConstructor {
+  PositiveFieldAfterConstructor() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F, G
+  int F;
+  // CHECK-FIXES: int F{};
+  bool G /* with comment */;
+  // CHECK-FIXES: bool G{} /* with comment */;
+  PositiveFieldBeforeConstructor IgnoredField;
+};
+
+struct PositiveSeparateDefinition {
+  PositiveSeparateDefinition();
+  int F;
+  // CHECK-FIXES: int F{};
+};
+
+PositiveSeparateDefinition::PositiveSeparateDefinition() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these built-in/pointer fields: F
+
+struct PositiveMixedFieldOrder {
+  PositiveMixedFieldOrder() : J(0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: I, K
+  int I;
+  // CHECK-FIXES: int I{};
+  int J;
+  int K;
+  // CHECK-FIXES: int K{};
+};
+
+struct NegativeFieldInitialized {
+  int F;
+
+  NegativeFieldInitialized() : F() {}
+};
+
+struct NegativeFieldInitializedInDefinition {
+  int F;
+
+  NegativeFieldInitializedInDefinition();
+};
+NegativeFieldInitializedInDefinition::NegativeFieldInitializedInDefinition() : F() {}
+
+
+struct NegativeInClassInitialized {
+  int F = 0;
+
+  NegativeInClassInitialized() {}
+};
+
+struct NegativeConstructorDelegated {
+  int F;
+
+  NegativeConstructorDelegated(int F) : F(F) {}
+  NegativeConstructorDelegated() : NegativeConstructorDelegated(0) {}
+};
+
+struct NegativeInitializedInBody {
+  NegativeInitializedInBody() { I = 0; }
+  int I;
+};
Index: test/clang-tidy/misc-uninitialized-field-cxx98.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-uninitialized-field-cxx98.cpp
@@ -0,0 +1,65 @@
+// RUN: %check_clang_tidy %s misc-uninitialized-field %t -- -- -std=c++98
+
+struct PositiveFieldBeforeConstructor {
+  int F;
+  PositiveFieldBeforeConstructor() /* some comment */ {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F
+  // CHECK-FIXES: PositiveFieldBeforeConstructor() : F() /* some comment */ {}
+};
+
+struct PositiveFieldAfterConstructor {
+  PositiveFieldAfterConstructor() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F, G, H
+  // CHECK-FIXES: PositiveFieldAfterConstructor() : F(), G(), H() {}
+  int F;
+  bool G /* with comment */;
+  int *H;
+  PositiveFieldBeforeConstructor IgnoredField;
+};
+
+struct PositiveSeparateDefinition {
+  PositiveSeparateDefinition();
+  int F;
+};
+
+PositiveSeparateDefinition::PositiveSeparateDefinition() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these built-in/pointer fields: F
+// CHECK-FIXES: PositiveSeparateDefinition::PositiveSeparateDefinition() : F() {}
+
+struct PositiveMixedFieldOrder {
+  PositiveMixedFieldOrder() : /* some comment */ J(0), L(0), M(0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: I, K, N
+  // CHECK-FIXES: PositiveMixedFieldOrder() : I(), /* some comment */ J(0), K(), L(0), M(0), N() {}
+  int I;
+  int J;
+  int K;
+  int L;
+  int M;
+  int N;
+};
+
+struct PositiveAfterBaseInitializer : public PositiveMixedFieldOrder {
+  PositiveAfterBaseInitializer() : PositiveMixedFieldOrder() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F
+  // CHECK-FIXES: PositiveAfterBaseInitializer() : PositiveMixedFieldOrder(), F() {}
+  int F;
+};
+
+struct NegativeFieldInitialized {
+  int F;
+
+  NegativeFieldInitialized() : F() {}
+};
+
+struct NegativeFieldInitializedInDefinition {
+  int F;
+
+  NegativeFieldInitializedInDefinition();
+};
+
+NegativeFieldInitializedInDefinition::NegativeFieldInitializedInDefinition() : F() {}
+
+struct NegativeInitializedInBody {
+  NegativeInitializedInBody() { I = 0; }
+  int I;
+};
Index: docs/clang-tidy/checks/misc-uninitialized-field.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-uninitialized-field.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - misc-uninitialized-field
+
+misc-uninitialized-field
+==========================
+
+
+The check flags user-defined constructor definitions that don't initialize all
+builtin and pointer fields which leaves their memory in an undefined state.
+
+For C++11 it suggests fixes to add in-class field initializers. For older
+versions it inserts the field initializers into the constructor initializer
+list.
+
+The check takes assignment of fields in the constructor body into account but
+generates false positives for fields initialized in methods invoked in the
+constructor body.
Index: clang-tidy/misc/UninitializedFieldCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/UninitializedFieldCheck.h
@@ -0,0 +1,40 @@
+//===--- UninitializedFieldCheck.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_MISC_UNINITIALIZED_FIELD_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNINITIALIZED_FIELD_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \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/misc-uninitialized-field.html
+/// TODO: See if 'fixes' for false positives are optimized away by the
+/// compiler.
+class UninitializedFieldCheck : public ClangTidyCheck {
+public:
+  UninitializedFieldCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNINITIALIZED_FIELD_H
+
Index: clang-tidy/misc/UninitializedFieldCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/UninitializedFieldCheck.cpp
@@ -0,0 +1,222 @@
+//===--- UninitializedFieldCheck.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 "UninitializedFieldCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <unordered_set>
+
+#include <iostream>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+namespace {
+
+static bool fieldRequiresInit(const FieldDecl *F) {
+  return F->getType()->isPointerType() || F->getType()->isBuiltinType();
+}
+
+void removeFieldsInitializedInBody(
+    const Stmt &Stmt, ASTContext &Context,
+    std::unordered_set<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 std::unordered_set<const FieldDecl *> &FieldsRequiringInit) {
+  std::string List;
+  llvm::raw_string_ostream Stream(List);
+  size_t AddedFields = 0;
+  for (const FieldDecl *Field : FieldRange) {
+    if (FieldsRequiringInit.find(Field) != FieldsRequiringInit.end()) {
+      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;
+  std::vector<const FieldDecl *> Fields;
+
+  SourceLocation getLocation(const ASTContext &Context,
+                             const CXXConstructorDecl &Constructor) const {
+    if (InitializerBefore != nullptr)
+      return InitializerBefore->getRParenLoc().getLocWithOffset(1);
+    auto StartLocation = InitializerAfter != nullptr
+                             ? InitializerAfter->getSourceRange().getBegin()
+                             : Constructor.getBody()->getLocStart();
+    auto Token =
+        lexer_utils::getPreviousNonCommentToken(Context, StartLocation);
+    return Token.getLocation().getLocWithOffset(Token.getLength());
+  }
+
+  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() << "()";
+      }
+    }
+    Code = Stream.str();
+    // The initializer list is created, replace leading comma with colon.
+    if (InitializerBefore == nullptr && InitializerAfter == nullptr) {
+      Code[1] = ':';
+    }
+    return Code;
+  }
+};
+
+std::vector<FieldsInsertion> computeInsertions(
+    const CXXConstructorDecl::init_const_range &Inits,
+    const RecordDecl::field_range &Fields,
+    const std::unordered_set<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;
+  }
+  std::vector<FieldsInsertion> 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.find(*CurrentField) !=
+            FieldsRequiringInit.end())
+          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.find(*CurrentField) != FieldsRequiringInit.end())
+      OrderedFields.back().Fields.push_back(*CurrentField);
+  }
+  return OrderedFields;
+}
+
+} // namespace
+
+void UninitializedFieldCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(cxxConstructorDecl(isDefinition()).bind("ctor"), this);
+}
+
+void UninitializedFieldCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
+  if (!Ctor->isUserProvided())
+    return;
+
+  auto ParentClass = Ctor->getParent();
+  const auto &MemberFields = ParentClass->fields();
+
+  std::unordered_set<const FieldDecl *> FieldsToInit;
+  std::copy_if(MemberFields.begin(), MemberFields.end(),
+               std::inserter(FieldsToInit, FieldsToInit.end()),
+               &fieldRequiresInit);
+  if (FieldsToInit.empty())
+    return;
+
+  for (CXXCtorInitializer *Init : Ctor->inits()) {
+    if (Init->isDelegatingInitializer())
+      return;
+    if (!Init->isMemberInitializer())
+      continue;
+    const FieldDecl *MemberField = Init->getMember();
+    FieldsToInit.erase(MemberField);
+  }
+  removeFieldsInitializedInBody(*Ctor->getBody(), *Result.Context,
+                                FieldsToInit);
+
+  if (FieldsToInit.empty())
+    return;
+
+  // For C+11 use in-class initialization which covers all future constructors
+  // as well.
+  if (Result.Context->getLangOpts().CPlusPlus11) {
+    DiagnosticBuilder builder =
+        diag(
+            Ctor->getLocStart(),
+            "constructor does not initialize these built-in/pointer fields: %0")
+        << toCommaSeparatedString(MemberFields, FieldsToInit);
+
+    for (const auto *Field : FieldsToInit)
+      builder << FixItHint::CreateInsertion(
+          Field->getSourceRange().getEnd().getLocWithOffset(
+              Field->getName().size()),
+          "{}");
+    return;
+  }
+
+  DiagnosticBuilder builder =
+      diag(Ctor->getLocStart(),
+           "constructor does not initialize these built-in/pointer fields: %0")
+      << toCommaSeparatedString(MemberFields, FieldsToInit);
+  for (const auto &FieldsInsertion :
+       computeInsertions(Ctor->inits(), MemberFields, FieldsToInit))
+    if (!FieldsInsertion.Fields.empty())
+      builder << FixItHint::CreateInsertion(
+          FieldsInsertion.getLocation(*Result.Context, *Ctor),
+          FieldsInsertion.codeToInsert());
+}
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -29,6 +29,7 @@
 #include "SwappedArgumentsCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UndelegatedConstructor.h"
+#include "UninitializedFieldCheck.h"
 #include "UniqueptrResetReleaseCheck.h"
 #include "UnusedAliasDeclsCheck.h"
 #include "UnusedParametersCheck.h"
@@ -77,6 +78,8 @@
         "misc-throw-by-value-catch-by-reference");
     CheckFactories.registerCheck<UndelegatedConstructorCheck>(
         "misc-undelegated-constructor");
+    CheckFactories.registerCheck<UninitializedFieldCheck>(
+        "misc-uninitialized-field");
     CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
         "misc-uniqueptr-reset-release");
     CheckFactories.registerCheck<UnusedAliasDeclsCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -21,6 +21,7 @@
   SwappedArgumentsCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp
   UndelegatedConstructor.cpp
+  UninitializedFieldCheck.cpp
   UnusedAliasDeclsCheck.cpp
   UnusedParametersCheck.cpp
   UnusedRAIICheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to