flx updated this revision to Diff 45838.
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,220 @@
+//===--- 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>
+
+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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
