Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.

[michael_miller]

Summary

Added the remaining features needed to satisfy C++ Core Guideline Type.6: Always initialize a member variable to cppcoreguidelines-pro-type-member-init. The check now flags all default-constructed uses of record types without user-provided default constructors that would leave their memory in an undefined state. The check suggests value initializing them instead.

[LegalizeAdulthood]

Can you summarize the improvements in docs/ReleaseNotes.rst, please?

[michael_miller]

Added explanation of changes to ReleaseNotes.rst and cleaned up documentation for the check a little.

[alexfh]

Thank you for the patch! I'd like the original author to take a look at it, so just a couple of peanut gallery comments from me for now.

clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

51 ↗

(On Diff #51999)

I wonder whether some relevant parts of SemaExprCXX.cpp could be made reusable so that you don't have to completely reimplement those.

56 ↗

(On Diff #51999)

Please clang-format the code.

[michael_miller]

Ran clang-format on all modified source files.

[michael_miller]

Fixed tests after clang-format.
Fixed checks related to non-trivial compiler-generated default constructors that weren't accounted for in the previous iteration.

[aaron.ballman]

clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

25 ↗

(On Diff #52272)

Please do not remove the unnamed namespace; it is preferable to using static functions.

30 ↗

(On Diff #52272)

Missing a period at the end of the sentence.

33 ↗

(On Diff #52272)

This comment doesn't match the behavior of the code below.

104 ↗

(On Diff #52272)

Please use && instead of and.

152 ↗

(On Diff #52272)

This should probably be made more generally available as an AST matcher.

156 ↗

(On Diff #52272)

I think this matcher logic should go into utils\TypeTraits.cpp as it seems more generally useful than just this check.

160 ↗

(On Diff #52272)

As should this.

187 ↗

(On Diff #52272)

Please use || instead of or. Also, asserts should usually have && "Some explanatory text" for when the assert is triggered.

243 ↗

(On Diff #52272)

Since this function is only used once, I think it should be lowered into its use as a ternary conditional.

256 ↗

(On Diff #52272)

Please do not use an initializer list like this, I believe it won't work in MSVC 2013. (Here and elsewhere as well.)

258 ↗

(On Diff #52272)

Please only use auto when the type name is spelled out as part of the initializer, or it is used as a for-range initializer.

296 ↗

(On Diff #52272)

Can use const auto & here instead.

323 ↗

(On Diff #52272)

Since this is a C++ core guideline, I think the matchers should only be registered when compiling for C++. It may make sense to use a similar check in C mode, but we can cross that bridge when we come to it (and have tests for it).

486 ↗

(On Diff #52272)

I think this code belongs in fixInitializerList then, doesn't it?

497 ↗

(On Diff #52272)

You should be able to pass in Var directly.

test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

285 ↗

(On Diff #52272)

s/on/one

291 ↗

(On Diff #52272)

s/on/one

334 ↗

(On Diff #52272)

I'd like to see how we handle the following test:

struct S {
  S(int i);
};
struct T {
  T(float f);
};

union U {
  S s;
  T t;
};

[michael_miller]

Addressed comments.
Moved isTriviallyDefaultConstructible into utils/TypeTraits.
Moved AST matchers into utils/Matchers.h.
Enclosed all source-local functions in an anonymous namespace and removed static.
Replaced stray ands and ors with && and ||.

[alexfh]

A couple of nits.

clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

181 ↗

(On Diff #52752)

If you're doing this to disambiguate the ternary operator, you can skip the second cast.

test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

285 ↗

(On Diff #52752)

nit: Trailing period is missing.

Same below.

[michael_miller]

Removed a superfluous cast and added periods to some comments.

[alexfh]

Looks good from my side. Please wait for the Aaron's approval.

Thank you for working on this!

[michael_miller]

In D18584#395208, @alexfh wrote:

Looks good from my side. Please wait for the Aaron's approval.

Thank you for working on this!

No problem! Thanks for taking the time to review and give feedback. clang-tidy is such a great tool. It's my honor and pleasure to be able to contribute and give back even a little!

Edit: Huh. Didn't realize those comments I had written didn't get posted. I'm new to Phabricator...

clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

25 ↗

(On Diff #52272)

Ok, I can put the anonymous namespace around everything and remove the static specifiers. I moved it down lower to enclose the local class definition because it wasn't previously. I would normally wrap the whole thing but the LLVM coding standards seem to say the opposite of what I'm used to doing with regard to static functions vs anonymous namespaces:
http://llvm.org/docs/CodingStandards.html#anonymous-namespaces

51 ↗

(On Diff #52272)

I was wondering about that too but didn't really know how to proceed. I did come across SemaExprCXX.cpp while looking to see how std::is_trivially_default_constructible was implemented. Being unfamiliar with that part of the codebase, though, I didn't dig too far into how to either share that code or hook into it.

243 ↗

(On Diff #52272)

I've gone ahead and done this but I think it's less readable afterwards. The reason it's problematic is because the return types from the two subexpressions aren't the same so it can't be used as a ternary conditional without a cast.

486 ↗

(On Diff #52272)

Sure. I think one reason I didn't was because of line 426 which is structured similarly. Stylistically, I slightly prefer to test the condition explicitly rather than have an early return but it's perfectly fine either way.

test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

334 ↗

(On Diff #52272)

For a number of reasons, it doesn't hit any warnings:

1. We don't do anything to record types without any members.
2. U doesn't have a user-provided constructor. We only initialize members if there's a user-provided constructor of some sort.
3. If we declared an instance of U as a local variable, we would have to explicitly initialize it. The default constructor is implicitly deleted because of S and T.

[aaron.ballman]

clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

157 ↗

(On Diff #52950)

getCanonicalRecordDecl() instead? (Otherwise, this suggests the function could be replaced with Type->getCXXRecordDecl() at the call site, but that's not quite right because this function canonicalizes the type first.)

clang-tidy/utils/Matchers.h

31 ↗

(On Diff #52950)

This should go into clang\include\clang\ASTMatchers\ASTMatchers.h instead. When added there, it should get doxygen comments, appropriate test cases in clang\unittests\ASTMatchers\ASTMatchersTest.cpp, and then you should regenerate the online documentation by running clang\docs\tools\dump_ast_matchers.py.

35 ↗

(On Diff #52950)

Same for this one.

[michael_miller]

Moved isDelegatingConstructor and isUserProvided into clang/include/clang/ASTMatchers/ASTMatchers.h and updated the tests and docs.
Renamed getRecordDecl to getCanonicalRecordDecl to better match its behavior.

[michael_miller]

Sounds good! Updated with suggested changes.

[aaron.ballman]

LGTM with one minor correction (you can correct when committing it).

unittests/ASTMatchers/ASTMatchersTest.cpp

2343 ↗

(On Diff #53380)

I think this test got duplicated accidentally; I don't see it removed as part of the diff.

[michael_miller]

Removed accidentally duplicated test case.

[michael_miller]

In D18584#398298, @aaron.ballman wrote:

LGTM with one minor correction (you can correct when committing it).

Oops! Not sure how that happened. I submitted a revised diff.

[michael_miller]

In D18584#398298, @aaron.ballman wrote:

LGTM with one minor correction (you can correct when committing it).

Also, what's the next step? Is there anything else for me to do?

[alexfh]

Do you have svn commit access?

[michael_miller]

In D18584#398929, @alexfh wrote:

Do you have svn commit access?

I do not... this is my first contribution to llvm/clang.

[alexfh]

I can commit the patch for you, but you need to split it in two: one for the cfe repository, one for clang-tools-extra.

[michael_miller]

In D18584#399024, @alexfh wrote:

I can commit the patch for you, but you need to split it in two: one for the cfe repository, one for clang-tools-extra.

Great—thanks! I have them separated already and had to merge them for the Phabricator review. Does that mean I should open another review for the cfe one with you as a reviewer and make this one only clang-tools-extra changes or is there an easier way for me to give the patches to you?

[alexfh]

In D18584#399056, @michael_miller wrote:

In D18584#399024, @alexfh wrote:

I can commit the patch for you, but you need to split it in two: one for the cfe repository, one for clang-tools-extra.

Great—thanks! I have them separated already and had to merge them for the Phabricator review. Does that mean I should open another review for the cfe one with you as a reviewer and make this one only clang-tools-extra changes or is there an easier way for me to give the patches to you?

Yes, please create another Differential revision for the cfe patch. That's probably the easiest way to transfer patches ;)

[michael_miller]

Broke out the cfe parts into its own change (D19038).

[alexfh]

FYI, the check has started crashing after this patch. I'll try to provide a minimal test case soon. The relevant part of the stack trace is:

@     0x7fc9c255efa0          8  clang::Stmt::getLocStart()
@     0x7fc9c48fdac1         64  clang::tidy::cppcoreguidelines::(anonymous namespace)::IntializerInsertion::getLocation()
@     0x7fc9c49026d5       1696  clang::tidy::cppcoreguidelines::ProTypeMemberInitCheck::checkMissingBaseClassInitializer()
@     0x7fc9c490424f         96  clang::tidy::cppcoreguidelines::ProTypeMemberInitCheck::check()

[michael_miller]

In D18584#403872, @alexfh wrote:

FYI, the check has started crashing after this patch. I'll try to provide a minimal test case soon. The relevant part of the stack trace is:

@     0x7fc9c255efa0          8  clang::Stmt::getLocStart()
@     0x7fc9c48fdac1         64  clang::tidy::cppcoreguidelines::(anonymous namespace)::IntializerInsertion::getLocation()
@     0x7fc9c49026d5       1696  clang::tidy::cppcoreguidelines::ProTypeMemberInitCheck::checkMissingBaseClassInitializer()
@     0x7fc9c490424f         96  clang::tidy::cppcoreguidelines::ProTypeMemberInitCheck::check()

Hmm... I can get this to crash in that location but only if I run with -fdelayed-template-parsing. I've got an easy fix and test case if that's the same issue you're running into. We should move the check at line 307 into ProTypeMemberInitCheck::check before both checks. I've got a test case I can add to cppcoreguidelines-pro-type-member-init-delayed.cpp that reproduces it. Let me know if I should write up a patch for this.

[alexfh]

Would be nice, if you could write a patch. I don't yet have a reduced test
case (it crashes without delayed template parsing), since my creduce is
still running after many hours ;)

[hokein]

In D18584#404192, @michael_miller wrote:

In D18584#403872, @alexfh wrote:

FYI, the check has started crashing after this patch. I'll try to provide a minimal test case soon. The relevant part of the stack trace is:

@     0x7fc9c255efa0          8  clang::Stmt::getLocStart()
@     0x7fc9c48fdac1         64  clang::tidy::cppcoreguidelines::(anonymous namespace)::IntializerInsertion::getLocation()
@     0x7fc9c49026d5       1696  clang::tidy::cppcoreguidelines::ProTypeMemberInitCheck::checkMissingBaseClassInitializer()
@     0x7fc9c490424f         96  clang::tidy::cppcoreguidelines::ProTypeMemberInitCheck::check()

Hmm... I can get this to crash in that location but only if I run with -fdelayed-template-parsing. I've got an easy fix and test case if that's the same issue you're running into. We should move the check at line 307 into ProTypeMemberInitCheck::check before both checks. I've got a test case I can add to cppcoreguidelines-pro-type-member-init-delayed.cpp that reproduces it. Let me know if I should write up a patch for this.

I have created a minimal test case, see https://llvm.org/bugs/show_bug.cgi?id=27419. I think it's related to template. It would be very nice if you can submit a patch fixing this (and include the crash test case).

[michael_miller]

In D18584#404901, @hokein wrote:

In D18584#404192, @michael_miller wrote:

In D18584#403872, @alexfh wrote:

FYI, the check has started crashing after this patch. I'll try to provide a minimal test case soon. The relevant part of the stack trace is:

@     0x7fc9c255efa0          8  clang::Stmt::getLocStart()
@     0x7fc9c48fdac1         64  clang::tidy::cppcoreguidelines::(anonymous namespace)::IntializerInsertion::getLocation()
@     0x7fc9c49026d5       1696  clang::tidy::cppcoreguidelines::ProTypeMemberInitCheck::checkMissingBaseClassInitializer()
@     0x7fc9c490424f         96  clang::tidy::cppcoreguidelines::ProTypeMemberInitCheck::check()

Hmm... I can get this to crash in that location but only if I run with -fdelayed-template-parsing. I've got an easy fix and test case if that's the same issue you're running into. We should move the check at line 307 into ProTypeMemberInitCheck::check before both checks. I've got a test case I can add to cppcoreguidelines-pro-type-member-init-delayed.cpp that reproduces it. Let me know if I should write up a patch for this.

I have created a minimal test case, see https://llvm.org/bugs/show_bug.cgi?id=27419. I think it's related to template. It would be very nice if you can submit a patch fixing this (and include the crash test case).

Thanks for the test case! The same fix I had applies. I reduced your test case down even further and added it to the tests. I submitted a patch with these fixes (D19270).

[chapuni]

This had been failing. Fixed in r267290.
See also http://bb.pgr.jp/builders/msbuild-llvmclang-x64-msc19-DA/builds/349

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

120

raw_string_ostream cannot be assumed as unbuffered, nor would be flushed by the destructor before Code copied to return value.

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.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.
+/// \brief Implements C++ Core Guidelines Type.6.
+///
+/// Checks that every user-provided constructor value-initializes all class
+/// members and base classes that would have undefined behavior otherwise. Also
+/// check that any record types without user-provided default constructors are
+/// value-initialized where used.
 ///
 /// 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 {}."
+/// TODO: For classes with multiple constructors, make sure that we don't offer
+///     multiple in-class initializer fixits for the same  member.
 class ProTypeMemberInitCheck : public ClangTidyCheck {
 public:
-  ProTypeMemberInitCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  ProTypeMemberInitCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  // Checks Type.6 part 1:
+  // Issue a diagnostic for any constructor of a non-trivially-constructible
+  // type that does not initialize all member variables.
+  //
+  // To fix: Write a data member initializer, or mention it in the member
+  // initializer list.
+  void checkMissingMemberInitializer(ASTContext &Context,
+                                     const CXXConstructorDecl *Ctor);
+
+  // A subtle side effect of Type.6 part 2:
+  // Make sure to initialize trivially constructible base classes.
+  void checkMissingBaseClassInitializer(const ASTContext &Context,
+                                        const CXXConstructorDecl *Ctor);
+
+  // Checks Type.6 part 2:
+  // Issue a diagnostic when constructing an object of a trivially constructible
+  // type without () or {} to initialize its members.
+  //
+  // To fix: Add () or {}.
+  void checkUninitializedTrivialType(const ASTContext &Context,
+                                     const VarDecl *Var);
+
+  // Whether arrays need to be initialized or not. Default is false.
+  bool IgnoreArrays;
 };
 
 } // namespace cppcoreguidelines
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

 //===--- 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 "../utils/Matchers.h"
+#include "../utils/TypeTraits.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 namespace clang::tidy::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,
+void fieldsRequiringInit(const RecordDecl::field_range &Fields,
+                         ASTContext &Context,
                          SmallPtrSetImpl<const FieldDecl *> &FieldsToInit) {
   for (const FieldDecl *F : Fields) {
     QualType Type = F->getType();
-    if (Type->isPointerType() || Type->isBuiltinType())
+    if (!F->hasInClassInitializer() &&
+        type_traits::isTriviallyDefaultConstructible(Type, Context))
       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
+StringRef getName(const FieldDecl *Field) { return Field->getName(); }
+
+StringRef getName(const RecordDecl *Record) {
+  // Get the typedef name if this is a C-style anonymous struct and typedef.
+  if (const TypedefNameDecl *Typedef = Record->getTypedefNameForAnonDecl())
+    return Typedef->getName();
+  return Record->getName();
+}
+
+// Creates comma separated list of decls 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:
+template <typename R, typename T>
+std::string
+toCommaSeparatedString(const R &OrderedDecls,
+                       const SmallPtrSetImpl<const T *> &DeclsToInit) {
+  SmallVector<StringRef, 16> Names;
+  for (const T *Decl : OrderedDecls) {
+    if (DeclsToInit.count(Decl))
+      Names.emplace_back(getName(Decl));
+  }
+  return llvm::join(Names.begin(), Names.end(), ", ");
+}
+
+SourceLocation getLocationForEndOfToken(const ASTContext &Context,
+                                        SourceLocation Location) {
+  return Lexer::getLocForEndOfToken(Location, 0, Context.getSourceManager(),
+                                    Context.getLangOpts());
+};
+
+// There are 3 kinds of insertion placements:
+enum class InitializerPlacement {
   // 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.
+  // Where. This will be the case whenever there is a written initializer before
+  // the fields available.
+  After,
+
   // 2. The fields are inserted before the first existing initializer stored in
-// InitializerAfter.
+  // Where.
+  Before,
+
   // 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;
+  New
+};
+
+// An InitializerInsertion contains a list of fields and/or base classes to
+// insert into the initializer list of a constructor. We use this to ensure
+// proper absolute ordering according to the class declaration relative to the
+// (perhaps improper) ordering in the existing initializer list, if any.
+struct IntializerInsertion {
+  IntializerInsertion(InitializerPlacement Placement,
+                      const CXXCtorInitializer *Where)
+      : Placement(Placement), Where(Where) {}
 
   SourceLocation getLocation(const ASTContext &Context,
                              const CXXConstructorDecl &Constructor) const {
-    if (InitializerBefore != nullptr) {
-      return Lexer::getLocForEndOfToken(InitializerBefore->getRParenLoc(), 0,
-                                        Context.getSourceManager(),
-                                        Context.getLangOpts());
+    assert((Where != nullptr || Placement == InitializerPlacement::New) &&
+           "Location should be relative to an existing initializer or this "
+           "insertion represents a new initializer list.");
+    SourceLocation Location;
+    switch (Placement) {
+    case InitializerPlacement::New:
+      Location = lexer_utils::getPreviousNonCommentToken(
+                     Context, Constructor.getBody()->getLocStart())
+                     .getLocation();
+      break;
+    case InitializerPlacement::Before:
+      Location = lexer_utils::getPreviousNonCommentToken(
+                     Context, Where->getSourceRange().getBegin())
+                     .getLocation();
+      break;
+    case InitializerPlacement::After:
+      Location = Where->getRParenLoc();
+      break;
     }
-    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());
+    return getLocationForEndOfToken(Context, Location);
   }
 
   std::string codeToInsert() const {
-    assert(!Fields.empty() && "No fields to insert");
+    assert(!Initializers.empty() && "No initializers 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] = ':';
+    std::string joined =
+        llvm::join(Initializers.begin(), Initializers.end(), "(), ");
+    switch (Placement) {
+    case InitializerPlacement::New:
+      Stream << " : " << joined << "()";
+      break;
+    case InitializerPlacement::Before:
+      Stream << " " << joined << "(),";
+      break;
+    case InitializerPlacement::After:
+      Stream << ", " << joined << "()";
+      break;
+    }
     return Code;
   }
+
+  InitializerPlacement Placement;
+  const CXXCtorInitializer *Where;
+  SmallVector<std::string, 4> Initializers;
 };
 
-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;
+// Convenience utility to get a RecordDecl from a QualType.
+const RecordDecl *getCanonicalRecordDecl(const QualType &Type) {
+  if (const auto *RT = Type.getCanonicalType()->getAs<RecordType>())
+    return RT->getDecl();
+  return nullptr;
+}
+
+template <typename R, typename T>
+SmallVector<IntializerInsertion, 16>
+computeInsertions(const CXXConstructorDecl::init_const_range &Inits,
+                  const R &OrderedDecls,
+                  const SmallPtrSetImpl<const T *> &DeclsToInit) {
+  SmallVector<IntializerInsertion, 16> Insertions;
+  Insertions.emplace_back(InitializerPlacement::New, nullptr);
+
+  typename R::const_iterator Decl = std::begin(OrderedDecls);
   for (const CXXCtorInitializer *Init : Inits) {
-    if (Init->isWritten() && !Init->isMemberInitializer())
-      LastWrittenNonMemberInit = Init;
+    if (Init->isWritten()) {
+      if (Insertions.size() == 1)
+        Insertions.emplace_back(InitializerPlacement::Before, Init);
+
+      // Gets either the field or base class being initialized by the provided
+      // initializer.
+      const auto *InitDecl =
+          Init->isMemberInitializer()
+              ? static_cast<const NamedDecl *>(Init->getMember())
+              : Init->getBaseClass()->getAs<RecordType>()->getDecl();
+
+      // Add all fields between current field up until the next intializer.
+      for (; Decl != std::end(OrderedDecls) && *Decl != InitDecl; ++Decl) {
+        if (const T *D = dyn_cast<T>(*Decl)) {
+          if (DeclsToInit.count(D) > 0)
+            Insertions.back().Initializers.emplace_back(getName(D));
+        }
       }
-  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);
+      Insertions.emplace_back(InitializerPlacement::After, Init);
+    }
+  }
+
+  // Add remaining decls that require initialization.
+  for (; Decl != std::end(OrderedDecls); ++Decl) {
+    if (const T *D = dyn_cast<T>(*Decl)) {
+      if (DeclsToInit.count(D) > 0)
+        Insertions.back().Initializers.emplace_back(getName(D));
+    }
+  return Insertions;
+}
+
+// Gets the list of bases and members that could possibly be initialized, in
+// order as they appear in the class declaration.
+void getInitializationsInOrder(const CXXRecordDecl *ClassDecl,
+                               SmallVectorImpl<const NamedDecl *> &Decls) {
+  Decls.clear();
+  for (const auto &Base : ClassDecl->bases())
+    Decls.emplace_back(getCanonicalRecordDecl(Base.getType()));
+  Decls.append(ClassDecl->fields().begin(), ClassDecl->fields().end());
+}
+
+template <typename T>
+void fixInitializerList(const ASTContext &Context, DiagnosticBuilder &Diag,
+                        const CXXConstructorDecl *Ctor,
+                        const SmallPtrSetImpl<const T *> &DeclsToInit) {
+  // Do not propose fixes in macros since we cannot place them correctly.
+  if (Ctor->getLocStart().isMacroID())
+    return;
+
+  SmallVector<const NamedDecl *, 16> OrderedDecls;
+  getInitializationsInOrder(Ctor->getParent(), OrderedDecls);
+
+  for (const auto &Insertion :
+       computeInsertions(Ctor->inits(), OrderedDecls, DeclsToInit)) {
+    if (!Insertion.Initializers.empty())
+      Diag << FixItHint::CreateInsertion(Insertion.getLocation(Context, *Ctor),
+                                         Insertion.codeToInsert());
+  }
+}
+
+template <typename T, typename Func>
+void forEachField(const RecordDecl *Record, const T &Fields,
+                  bool OneFieldPerUnion, Func &&Fn) {
+  for (const FieldDecl *F : Fields) {
+    if (F->isAnonymousStructOrUnion()) {
+      if (const RecordDecl *R = getCanonicalRecordDecl(F->getType()))
+        forEachField(R, R->fields(), OneFieldPerUnion, Fn);
+    } else {
+      Fn(F);
+    }
+
+    if (OneFieldPerUnion && Record->isUnion())
+      break;
   }
-  return OrderedFields;
 }
 
 } // namespace
 
+ProTypeMemberInitCheck::ProTypeMemberInitCheck(StringRef Name,
+                                               ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IgnoreArrays(Options.get("IgnoreArrays", false)) {}
+
 void ProTypeMemberInitCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(cxxConstructorDecl(isDefinition(), isUserProvided(),
-                                        unless(isInstantiated()))
-                         .bind("ctor"),
-                     this);
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  auto IsUserProvidedNonDelegatingConstructor =
+      allOf(isUserProvided(),
+            unless(anyOf(isInstantiated(), isDelegatingConstructor())));
+  auto IsNonTrivialDefaultConstructor = allOf(
+      isDefaultConstructor(), unless(isUserProvided()),
+      hasParent(cxxRecordDecl(unless(isTriviallyDefaultConstructible()))));
+  Finder->addMatcher(
+      cxxConstructorDecl(isDefinition(),
+                         anyOf(IsUserProvidedNonDelegatingConstructor,
+                               IsNonTrivialDefaultConstructor))
+  auto HasDefaultConstructor = hasInitializer(
+      cxxConstructExpr(unless(requiresZeroInitialization()),
+                       hasDeclaration(cxxConstructorDecl(
+                           isDefaultConstructor(), unless(isUserProvided())))));
+  Finder->addMatcher(
+      varDecl(isDefinition(), HasDefaultConstructor,
+              hasType(recordDecl(has(fieldDecl()),
+                                 isTriviallyDefaultConstructible())))
+          .bind("var"),
+      this);
 }
 
 void ProTypeMemberInitCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
-  const auto &MemberFields = Ctor->getParent()->fields();
+  if (const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor")) {
+    checkMissingMemberInitializer(*Result.Context, Ctor);
+    checkMissingBaseClassInitializer(*Result.Context, Ctor);
+  } else if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var")) {
+    checkUninitializedTrivialType(*Result.Context, Var);
+  }
+}
+
+void ProTypeMemberInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreArrays", IgnoreArrays);
+}
+
+void ProTypeMemberInitCheck::checkMissingMemberInitializer(
+    ASTContext &Context, const CXXConstructorDecl *Ctor) {
+  const CXXRecordDecl *ClassDecl = Ctor->getParent();
+  bool IsUnion = ClassDecl->isUnion();
+
+  if (IsUnion && ClassDecl->hasInClassInitializer())
+    return;
 
   // Skip declarations delayed by late template parsing without a body.
   const Stmt *Body = Ctor->getBody();
   if (!Body)
     return;
 
   SmallPtrSet<const FieldDecl *, 16> FieldsToInit;
-  fieldsRequiringInit(MemberFields, FieldsToInit);
+  fieldsRequiringInit(ClassDecl->fields(), Context, 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;
+  for (const CXXCtorInitializer *Init : Ctor->inits()) {
+    // Remove any fields that were explicitly written in the initializer list
+    // or in-class.
+    if (Init->isAnyMemberInitializer() && Init->isWritten()) {
+      if (IsUnion)
+        return; // We can only initialize one member of a union.
       FieldsToInit.erase(Init->getMember());
     }
-  removeFieldsInitializedInBody(*Body, *Result.Context, FieldsToInit);
+  }
+  removeFieldsInitializedInBody(*Body, Context, FieldsToInit);
 
-  if (FieldsToInit.empty())
+  // Collect all fields in order, both direct fields and indirect fields from
+  // anonmyous record types.
+  SmallVector<const FieldDecl *, 16> OrderedFields;
+  forEachField(ClassDecl, ClassDecl->fields(), false,
+               [&](const FieldDecl *F) { OrderedFields.push_back(F); });
+
+  // Collect all the fields we need to initialize, including indirect fields.
+  SmallPtrSet<const FieldDecl *, 16> AllFieldsToInit;
+  forEachField(ClassDecl, FieldsToInit, false,
+               [&](const FieldDecl *F) { AllFieldsToInit.insert(F); });
+  if (AllFieldsToInit.empty())
     return;
 
   DiagnosticBuilder Diag =
       diag(Ctor->getLocStart(),
-           "constructor does not initialize these built-in/pointer fields: %0")
-      << toCommaSeparatedString(MemberFields, FieldsToInit);
+           IsUnion
+               ? "union constructor should initialize one of these fields: %0"
+               : "constructor does not initialize these fields: %0")
+      << toCommaSeparatedString(OrderedFields, AllFieldsToInit);
+
   // 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) {
+
+  // Collect all fields but only suggest a fix for the first member of unions,
+  // as initializing more than one union member is an error.
+  SmallPtrSet<const FieldDecl *, 16> FieldsToFix;
+  forEachField(ClassDecl, FieldsToInit, true, [&](const FieldDecl *F) {
+    // Don't suggest fixes for enums because we don't know a good default.
+    if (!F->getType()->isEnumeralType())
+      FieldsToFix.insert(F);
+  });
+  if (FieldsToFix.empty())
+    return;
+
+  // Use in-class initialization if possible.
+  if (Context.getLangOpts().CPlusPlus11) {
+    for (const FieldDecl *Field : FieldsToFix) {
       Diag << FixItHint::CreateInsertion(
-          Lexer::getLocForEndOfToken(Field->getSourceRange().getEnd(), 0,
-                                     Result.Context->getSourceManager(),
-                                     Result.Context->getLangOpts()),
+          getLocationForEndOfToken(Context, Field->getSourceRange().getEnd()),
           "{}");
     }
+  } else {
+    // Otherwise, rewrite the constructor's initializer list.
+    fixInitializerList(Context, Diag, Ctor, FieldsToFix);
+  }
+}
+
+void ProTypeMemberInitCheck::checkMissingBaseClassInitializer(
+    const ASTContext &Context, const CXXConstructorDecl *Ctor) {
+  const CXXRecordDecl *ClassDecl = Ctor->getParent();
+
+  // Gather any base classes that need to be initialized.
+  SmallVector<const RecordDecl *, 4> AllBases;
+  SmallPtrSet<const RecordDecl *, 4> BasesToInit;
+  for (const CXXBaseSpecifier &Base : ClassDecl->bases()) {
+    if (const auto *BaseClassDecl = getCanonicalRecordDecl(Base.getType())) {
+      AllBases.emplace_back(BaseClassDecl);
+      if (!BaseClassDecl->field_empty() &&
+          type_traits::isTriviallyDefaultConstructible(Base.getType(), Context))
+        BasesToInit.insert(BaseClassDecl);
+    }
+  }
+
+  if (BasesToInit.empty())
     return;
+
+  // Remove any bases that were explicitly written in the initializer list.
+  for (const CXXCtorInitializer *Init : Ctor->inits()) {
+    if (Init->isBaseInitializer() && Init->isWritten())
+      BasesToInit.erase(Init->getBaseClass()->getAs<RecordType>()->getDecl());
   }
-  for (const auto &FieldsInsertion :
-       computeInsertions(Ctor->inits(), MemberFields, FieldsToInit)) {
-    if (!FieldsInsertion.Fields.empty())
-      Diag << FixItHint::CreateInsertion(
-          FieldsInsertion.getLocation(*Result.Context, *Ctor),
-          FieldsInsertion.codeToInsert());
-  }
+
+  if (BasesToInit.empty())
+    return;
+
+  DiagnosticBuilder Diag =
+      diag(Ctor->getLocStart(),
+           "constructor does not initialize these bases: %0")
+      << toCommaSeparatedString(AllBases, BasesToInit);
+
+  fixInitializerList(Context, Diag, Ctor, BasesToInit);
+
+void ProTypeMemberInitCheck::checkUninitializedTrivialType(
+    const ASTContext &Context, const VarDecl *Var) {
+  DiagnosticBuilder Diag =
+      diag(Var->getLocStart(), "uninitialized record type: %0") << Var;
+
+  Diag << FixItHint::CreateInsertion(
+      getLocationForEndOfToken(Context, Var->getSourceRange().getEnd()),
+      Context.getLangOpts().CPlusPlus11 ? "{}" : " = {}");
 }
 
 } // namespace cppcoreguidelines
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/clang-tidy/utils/Matchers.h

 namespace matchers {
 
 AST_MATCHER(QualType, isExpensiveToCopy) {
   llvm::Optional<bool> IsExpensive =
       type_traits::isExpensiveToCopy(Node, Finder->getASTContext());
   return IsExpensive && *IsExpensive;
 }
 
+AST_MATCHER(RecordDecl, isTriviallyDefaultConstructible) {
+  return type_traits::recordIsTriviallyDefaultConstructible(
+      Node, Finder->getASTContext());
+}
+
 } // namespace matchers
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_MATCHERS_H

clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.h

 
 namespace clang {
 namespace tidy {
 namespace type_traits {
 
 // \brief Returns true If \c Type is expensive to copy.
 llvm::Optional<bool> isExpensiveToCopy(QualType Type, ASTContext &Context);
 
+// \brief Returns true If \c Type is trivially default constructible.
+bool isTriviallyDefaultConstructible(QualType Type, const ASTContext &Context);
+
+// \brief Returns true If \c RecordDecl is trivially default constructible.
+bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl,
+                                           const ASTContext &Context);
+
 } // type_traits
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_TYPETRAITS_H

clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp

 
 llvm::Optional<bool> isExpensiveToCopy(QualType Type, ASTContext &Context) {
   if (Type->isDependentType())
     return llvm::None;
   return !Type.isTriviallyCopyableType(Context) &&
          !classHasTrivialCopyAndDestroy(Type);
 }
 
+bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl,
+                                           const ASTContext &Context) {
+  const auto *ClassDecl = dyn_cast<CXXRecordDecl>(&RecordDecl);
+  // Non-C++ records are always trivially constructible.
+  if (!ClassDecl)
+    return true;
+  // A class with a user-provided default constructor is not trivially
+  // constructible.
+  if (ClassDecl->hasUserProvidedDefaultConstructor())
+    return false;
+  // A class is trivially constructible if it has a trivial default constructor.
+  if (ClassDecl->hasTrivialDefaultConstructor())
+    return true;
+
+  // If all its fields are trivially constructible.
+  for (const FieldDecl *Field : ClassDecl->fields()) {
+    if (!isTriviallyDefaultConstructible(Field->getType(), Context))
+      return false;
+  }
+  // If all its direct bases are trivially constructible.
+  for (const CXXBaseSpecifier &Base : ClassDecl->bases()) {
+    if (!isTriviallyDefaultConstructible(Base.getType(), Context))
+      return false;
+  }
+
+  return true;
+}
+
+// Based on QualType::isTrivial.
+bool isTriviallyDefaultConstructible(QualType Type, const ASTContext &Context) {
+  if (Type.isNull())
+    return false;
+
+  if (Type->isArrayType())
+    return isTriviallyDefaultConstructible(Context.getBaseElementType(Type),
+                                           Context);
+
+  // Return false for incomplete types after skipping any incomplete array
+  // types which are expressly allowed by the standard and thus our API.
+  if (Type->isIncompleteType())
+    return false;
+
+  if (Context.getLangOpts().ObjCAutoRefCount) {
+    switch (Type.getObjCLifetime()) {
+    case Qualifiers::OCL_ExplicitNone:
+      return true;
+
+    case Qualifiers::OCL_Strong:
+    case Qualifiers::OCL_Weak:
+    case Qualifiers::OCL_Autoreleasing:
+      return false;
+
+    case Qualifiers::OCL_None:
+      if (Type->isObjCLifetimeType())
+        return false;
+      break;
+    }
+  }
+
+  QualType CanonicalType = Type.getCanonicalType();
+  if (CanonicalType->isDependentType())
+    return false;
+
+  // As an extension, Clang treats vector types as Scalar types.
+  if (CanonicalType->isScalarType() || CanonicalType->isVectorType())
+    return true;
+
+  if (const auto *RT = CanonicalType->getAs<RecordType>()) {
+    return recordIsTriviallyDefaultConstructible(*RT->getDecl(), Context);
+  }
+
+  // No other types can match.
+  return false;
+}
+
 } // type_traits
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/docs/ReleaseNotes.rst

* `readability-identifier-naming
     <http://llvm.org/releases/3.8.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-identifier-naming.html>`_
   * `readability-implicit-bool-cast
     <http://llvm.org/releases/3.8.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-implicit-bool-cast.html>`_
   * `readability-inconsistent-declaration-parameter-name
     <http://llvm.org/releases/3.8.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.html>`_
   * `readability-uniqueptr-delete-release
     <http://llvm.org/releases/3.8.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.html>`_
 
+- Updated ``cppcoreguidelines-pro-member-type-member-init`` check
+
+  This check now conforms to C++ Core Guidelines rule Type.6: Always Initialize
+  a Member Variable. The check examines every record type where construction
+  might result in an undefined memory state. These record types needing
+  initialization have at least one default-initialized built-in, pointer,
+  array or record type matching these criteria or a default-initialized
+  direct base class of this kind.
+
+  The check has two complementary aspects:
+  1. Ensure every constructor for a record type needing initialization
+     value-initializes all members and direct bases via a combination of
+     in-class initializers and the member initializer list.
+  2. Value-initialize every non-member instance of a record type needing
+     initialization that lacks a user-provided default constructor, e.g.
+     a POD.
 
 Improvements to modularize
 --------------------------
 
 The improvements are...

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst

 .. title:: clang-tidy - cppcoreguidelines-pro-type-member-init
 
 cppcoreguidelines-pro-type-member-init
 ======================================
 
-The check flags user-defined constructor definitions that do not initialize all
-builtin and pointer fields which leaves their memory in an undefined state.
+The check flags user-defined constructor definitions that do not
+initialize all fields that would be left in an undefined state by
+default construction, e.g. builtins, pointers and record types without
+user-provided default constructors containing at least one such
+type. If these fields aren't initialized, the constructor will leave
+some of the 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.
+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. It will also initialize any direct base classes that
+need to be zeroed in the constructor initializer list.
 
-This rule is part of the "Type safety" profile of the C++ Core Guidelines, see
+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.
+
+The check also flags variables of record types without a user-provided
+constructor that are not initialized. The suggested fix is to zero
+initialize the variable via {} for C++11 and beyond or = {} for older
+versions.
+
+IgnoreArrays option
+-------------------
+
+For performance critical code, it may be important to not zero
+fixed-size array members. If on, IgnoreArrays will not warn about
+array members that are not zero-initialized during construction.
+IgnoreArrays is false by default.
+
+This rule is part of the "Type safety" profile of the C++ Core
+Guidelines, corresponding to rule Type.6. See
 https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Pro-type-memberinit.

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp

 // RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init %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-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these 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-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these 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-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these 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-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these 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-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these 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;
 };
 
+struct NegativeAggregateType {
+  int X;
+  int Y;
+  int Z;
+};
+
+struct NonTrivialType {
+  int X;
+  int Y;
+};
+
+struct PositiveUninitializedBaseOrdering : public NegativeAggregateType,
+                                           public NonTrivialType {
+  PositiveUninitializedBaseOrdering() : NegativeAggregateType(), NonTrivialType(), B() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A
+  // CHECK-FIXES: PositiveUninitializedBaseOrdering() : NegativeAggregateType(), NonTrivialType(), A(), B() {}
+
+  // This is somewhat pathological with the base class initializer at the end...
+  PositiveUninitializedBaseOrdering(int) : B(), NonTrivialType(), A() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NegativeAggregateType
+  // CHECK-FIXES: PositiveUninitializedBaseOrdering(int) : B(), NegativeAggregateType(), NonTrivialType(), A() {}
+
+  PositiveUninitializedBaseOrdering(float) : NegativeAggregateType(), A() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NonTrivialType
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: constructor does not initialize these fields: B
+  // CHECK-FIXES: PositiveUninitializedBaseOrdering(float) : NegativeAggregateType(), NonTrivialType(), A(), B() {}
 
+  int A, B;
+};

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init-delayed.cpp

 // RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init %t -- -- -fdelayed-template-parsing
 
-template<class T>
+template <class T>
 struct PositiveFieldBeforeConstructor {
-  PositiveFieldBeforeConstructor() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F, G, H
   int F;
   bool G /* with comment */;
   int *H;
+  PositiveFieldBeforeConstructor() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: F, G, H
 };
 // Explicit instantiation.
 template class PositiveFieldBeforeConstructor<int>;
 
-template<class T>
+template <class T>
 struct PositiveFieldAfterConstructor {
   PositiveFieldAfterConstructor() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F, G, H
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: F, G, H
   int F;
   bool G /* with comment */;
   int *H;
 };
 // Explicit instantiation.
 template class PositiveFieldAfterConstructor<int>;
 
 // This declaration isn't used and won't be parsed 'delayed-template-parsing'.
 // The body of the declaration is 'null' and may cause crash if not handled
 // properly by checkers.
-template<class T>
+template <class T>
 struct UnusedDelayedConstructor {
   UnusedDelayedConstructor() {}
   int F;
 };

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

 // RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init %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
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: F
   // CHECK-FIXES: PositiveFieldBeforeConstructor() {}
 };
 
 struct PositiveFieldAfterConstructor {
   PositiveFieldAfterConstructor() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F, G
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: F, G
   // CHECK-FIXES: PositiveFieldAfterConstructor() {}
   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
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: F
 // CHECK-FIXES: PositiveSeparateDefinition::PositiveSeparateDefinition() {}
 
 struct PositiveMixedFieldOrder {
   PositiveMixedFieldOrder() : J(0) {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: I, K
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: I, K
   // CHECK-FIXES: PositiveMixedFieldOrder() : J(0) {}
   int I;
   // CHECK-FIXES: int I{};
   int J;
   int K;
   // CHECK-FIXES: int K{};
 };
 
 template <typename T>
 struct Template {
   Template() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: F
   int F;
   // CHECK-FIXES: int F{};
   T T1;
   // CHECK-FIXES: T T1;
 };
 
 void instantiate() {
   Template<int> TInt;
 }
 
 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;
 };
 
 #define UNINITIALIZED_FIELD_IN_MACRO_BODY(FIELD) \
-  struct UninitializedField##FIELD {     \
-    UninitializedField##FIELD() {}     \
-    int FIELD;           \
-  };             \
+  struct UninitializedField##FIELD {             \
+    UninitializedField##FIELD() {}               \
+    int FIELD;                                   \
+  };                                             \
 // Ensure FIELD is not initialized since fixes inside of macros are disabled.
 // CHECK-FIXES: int FIELD;
 
 UNINITIALIZED_FIELD_IN_MACRO_BODY(F);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these built-in/pointer fields: F
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: F
 UNINITIALIZED_FIELD_IN_MACRO_BODY(G);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these built-in/pointer fields: G
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: G
 
 #define UNINITIALIZED_FIELD_IN_MACRO_ARGUMENT(ARGUMENT) \
-  ARGUMENT            \
+  ARGUMENT
 
 UNINITIALIZED_FIELD_IN_MACRO_ARGUMENT(struct UninitializedFieldInMacroArg {
   UninitializedFieldInMacroArg() {}
   int Field;
 });
-// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: constructor does not initialize these built-in/pointer fields: Field
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: constructor does not initialize these fields: Field
 // Ensure FIELD is not initialized since fixes inside of macros are disabled.
 // CHECK-FIXES: int Field;
+
+struct NegativeAggregateType {
+  int X;
+  int Y;
+  int Z;
+};
+
+struct PositiveTrivialType {
+  PositiveTrivialType() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: F
+
+  NegativeAggregateType F;
+  // CHECK-FIXES: NegativeAggregateType F{};
+};
+
+struct NegativeNonTrivialType {
+  PositiveTrivialType F;
+};
+
+static void PositiveUninitializedTrivialType() {
+  NegativeAggregateType X;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: uninitialized record type: 'X'
+  // CHECK-FIXES: NegativeAggregateType X{};
+
+  NegativeAggregateType A[10]; // Don't warn because this isn't an object type.
+}
+
+static void NegativeInitializedTrivialType() {
+  NegativeAggregateType X{};
+  NegativeAggregateType Y = {};
+  NegativeAggregateType Z = NegativeAggregateType();
+  NegativeAggregateType A[10]{};
+  NegativeAggregateType B[10] = {};
+  int C; // No need to initialize this because we don't have a constructor.
+  int D[8];
+  NegativeAggregateType E = {0, 1, 2};
+  NegativeAggregateType F({});
+}
+
+struct NonTrivialType {
+  NonTrivialType() = default;
+  NonTrivialType(const NonTrivialType &RHS) : X(RHS.X), Y(RHS.Y) {}
+
+  int X;
+  int Y;
+};
+
+static void PositiveNonTrivialTypeWithCopyConstructor() {
+  NonTrivialType T;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: uninitialized record type: 'T'
+  // CHECK-FIXES: NonTrivialType T{};
+
+  NonTrivialType A[8];
+  // Don't warn because this isn't an object type
+}
+
+struct ComplexNonTrivialType {
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: constructor does not initialize these fields: Y
+  NegativeFieldInitialized X;
+  int Y;
+  // CHECK-FIXES: int Y{};
+};
+
+static void PositiveComplexNonTrivialType() {
+  ComplexNonTrivialType T;
+}
+
+struct PositiveStaticMember {
+  static NonTrivialType X;
+  static NonTrivialType Y;
+  static constexpr NonTrivialType Z{};
+};
+
+NonTrivialType PositiveStaticMember::X;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: uninitialized record type: 'X'
+// CHECK-FIXES: NonTrivialType PositiveStaticMember::X{};
+
+NonTrivialType PositiveStaticMember::Y{};
+
+struct PositiveMultipleConstructors {
+  PositiveMultipleConstructors() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A, B
+
+  PositiveMultipleConstructors(int) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A, B
+
+  PositiveMultipleConstructors(const PositiveMultipleConstructors &) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A, B
+
+  // FIXME: The fix-its here collide providing an erroneous fix
+  int A, B;
+  // CHECK-FIXES: int A{}{}{}, B{}{}{};
+};
+
+typedef struct {
+  int Member;
+} CStyleStruct;
+
+struct PositiveUninitializedBase : public NegativeAggregateType, CStyleStruct {
+  PositiveUninitializedBase() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NegativeAggregateType, CStyleStruct
+  // CHECK-FIXES: PositiveUninitializedBase() : NegativeAggregateType(), CStyleStruct() {}
+};
+
+struct PositiveUninitializedBaseOrdering : public NegativeAggregateType,
+                                           public NonTrivialType {
+  PositiveUninitializedBaseOrdering() : B() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NegativeAggregateType, NonTrivialType
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: constructor does not initialize these fields: A
+  // CHECK-FIXES: PositiveUninitializedBaseOrdering() : NegativeAggregateType(), NonTrivialType(), B() {}
+
+  // This is somewhat pathological with the base class initializer at the end...
+  PositiveUninitializedBaseOrdering(int) : B(), NonTrivialType(), A() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NegativeAggregateType
+  // CHECK-FIXES: PositiveUninitializedBaseOrdering(int) : B(), NegativeAggregateType(), NonTrivialType(), A() {}
+
+  PositiveUninitializedBaseOrdering(float) : B(), NegativeAggregateType(), A() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NonTrivialType
+  // CHECK-FIXES: PositiveUninitializedBaseOrdering(float) : B(), NegativeAggregateType(), NonTrivialType(), A() {}
+
+  int A, B;
+  // CHECK-FIXES: int A{}, B;
+};
+
+// We shouldn't need to initialize anything because PositiveUninitializedBase
+// has a user-defined constructor.
+struct NegativeUninitializedBase : public PositiveUninitializedBase {
+  NegativeUninitializedBase() {}
+};
+
+struct InheritedAggregate : public NegativeAggregateType {
+  int F;
+};
+
+static InheritedAggregate PositiveGlobal;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: uninitialized record type: 'PositiveGlobal'
+// CHECK-FIXES: InheritedAggregate PositiveGlobal{};
+
+enum TestEnum {
+  A,
+  B,
+  C
+};
+
+enum class TestScopedEnum {
+  A,
+  B,
+  C
+};
+
+struct PositiveEnumType {
+  PositiveEnumType() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: X, Y
+  // No proposed fixes, as we don't know whether value initialization for these
+  // enums really makes sense.
+
+  TestEnum X;
+  TestScopedEnum Y;
+};
+
+extern "C" {
+struct NegativeCStruct {
+  int X, Y, Z;
+};
+
+static void PositiveCStructVariable() {
+  NegativeCStruct X;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: uninitialized record type: 'X'
+  // CHECK-FIXES: NegativeCStruct X{};
+}
+}
+
+union NegativeUnionInClass {
+  NegativeUnionInClass() {} // No message as a union can only initialize one member.
+  int X = 0;
+  float Y;
+};
+
+union PositiveUnion {
+  PositiveUnion() : X() {} // No message as a union can only initialize one member.
+  PositiveUnion(int) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: union constructor should initialize one of these fields: X, Y
+
+  int X;
+  // CHECK-FIXES: int X{};
+
+  // Make sure we don't give Y an initializer.
+  float Y;
+  // CHECK-FIXES-NOT: float Y{};
+};
+
+struct PositiveAnonymousUnionAndStruct {
+  PositiveAnonymousUnionAndStruct() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A, B, Y, Z, C, D, E, F, X
+
+  union {
+    int A;
+    // CHECK-FIXES: int A{};
+    short B;
+  };
+
+  struct {
+    int Y;
+    // CHECK-FIXES: int Y{};
+    char *Z;
+    // CHECK-FIXES: char *Z{};
+
+    struct {
+      short C;
+      // CHECK-FIXES: short C{};
+      double D;
+      // CHECK-FIXES: double D{};
+    };
+
+    union {
+      long E;
+      // CHECK-FIXES: long E{};
+      float F;
+    };
+  };
+  int X;
+  // CHECK-FIXES: int X{};
+};