michael_miller updated this revision to Diff 53441.
michael_miller added a comment.
Removed accidentally duplicated test case.
http://reviews.llvm.org/D18584
Files:
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
clang-tidy/utils/Matchers.h
clang-tidy/utils/TypeTraits.cpp
clang-tidy/utils/TypeTraits.h
docs/LibASTMatchersReference.html
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
include/clang/ASTMatchers/ASTMatchers.h
test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp
test/clang-tidy/cppcoreguidelines-pro-type-member-init-delayed.cpp
test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
unittests/ASTMatchers/ASTMatchersTest.cpp
Index: unittests/ASTMatchers/ASTMatchersTest.cpp
===================================================================
--- unittests/ASTMatchers/ASTMatchersTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -2327,6 +2327,32 @@
cxxConstructorDecl(isMoveConstructor())));
}
+TEST(ConstructorDeclaration, IsUserProvided) {
+ EXPECT_TRUE(notMatches("struct S { int X = 0; };",
+ cxxConstructorDecl(isUserProvided())));
+ EXPECT_TRUE(notMatches("struct S { S() = default; };",
+ cxxConstructorDecl(isUserProvided())));
+ EXPECT_TRUE(notMatches("struct S { S() = delete; };",
+ cxxConstructorDecl(isUserProvided())));
+ EXPECT_TRUE(
+ matches("struct S { S(); };", cxxConstructorDecl(isUserProvided())));
+ EXPECT_TRUE(matches("struct S { S(); }; S::S(){}",
+ cxxConstructorDecl(isUserProvided())));
+}
+
+TEST(ConstructorDeclaration, IsDelegatingConstructor) {
+ EXPECT_TRUE(notMatches("struct S { S(); S(int); int X; };",
+ cxxConstructorDecl(isDelegatingConstructor())));
+ EXPECT_TRUE(notMatches("struct S { S(){} S(int X) : X(X) {} int X; };",
+ cxxConstructorDecl(isDelegatingConstructor())));
+ EXPECT_TRUE(matches(
+ "struct S { S() : S(0) {} S(int X) : X(X) {} int X; };",
+ cxxConstructorDecl(isDelegatingConstructor(), parameterCountIs(0))));
+ EXPECT_TRUE(matches(
+ "struct S { S(); S(int X); int X; }; S::S(int X) : S() {}",
+ cxxConstructorDecl(isDelegatingConstructor(), parameterCountIs(1))));
+}
+
TEST(DestructorDeclaration, MatchesVirtualDestructor) {
EXPECT_TRUE(matches("class Foo { virtual ~Foo(); };",
cxxDestructorDecl(ofClass(hasName("Foo")))));
Index: include/clang/ASTMatchers/ASTMatchers.h
===================================================================
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -4911,6 +4911,38 @@
return Node.isDefaultConstructor();
}
+/// \brief Matches constructor declarations that are user-provided.
+///
+/// Given
+/// \code
+/// struct S {
+/// S(); // #1
+/// S(const S &) = default; // #2
+/// S(S &&) = delete; // #3
+/// };
+/// \endcode
+/// cxxConstructorDecl(isUserProvided()) will match #1, but not #2 or #3.
+AST_MATCHER(CXXConstructorDecl, isUserProvided) {
+ return Node.isUserProvided();
+}
+
+/// \brief Matches constructors that delegate to another constructor.
+///
+/// Given
+/// \code
+/// struct S {
+/// S(); // #1
+/// S(int) {} // #2
+/// S(S &&) : S() {} // #3
+/// };
+/// S::S() : S(0) {} // #4
+/// \endcode
+/// cxxConstructorDecl(isDelegatingConstructor()) will match #3 and #4, but not
+/// #1 or #2.
+AST_MATCHER(CXXConstructorDecl, isDelegatingConstructor) {
+ return Node.isDelegatingConstructor();
+}
+
/// \brief Matches constructor and conversion declarations that are marked with
/// the explicit keyword.
///
Index: docs/LibASTMatchersReference.html
===================================================================
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -1799,6 +1799,21 @@
</pre></td></tr>
+<tr><td>Matcher<<a href="http://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html";>CXXConstructorDecl</a>></td><td class="name" onclick="toggle('isDelegatingConstructor0')"><a name="isDelegatingConstructor0Anchor">isDelegatingConstructor</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="isDelegatingConstructor0"><pre>Matches constructors that delegate to another constructor.
+
+Given
+ struct S {
+ S(); #1
+ S(int) {} #2
+ S(S &&) : S() {} #3
+ };
+ S::S() : S(0) {} #4
+cxxConstructorDecl(isDelegatingConstructor()) will match #3 and #4, but not
+#1 or #2.
+</pre></td></tr>
+
+
<tr><td>Matcher<<a href="http://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html";>CXXConstructorDecl</a>></td><td class="name" onclick="toggle('isExplicit0')"><a name="isExplicit0Anchor">isExplicit</a></td><td></td></tr>
<tr><td colspan="4" class="doc" id="isExplicit0"><pre>Matches constructor and conversion declarations that are marked with
the explicit keyword.
@@ -1828,6 +1843,19 @@
</pre></td></tr>
+<tr><td>Matcher<<a href="http://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html";>CXXConstructorDecl</a>></td><td class="name" onclick="toggle('isUserProvided0')"><a name="isUserProvided0Anchor">isUserProvided</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="isUserProvided0"><pre>Matches constructor declarations that are user-provided.
+
+Given
+ struct S {
+ S(); #1
+ S(const S &) = default; #2
+ S(S &&) = delete; #3
+ };
+cxxConstructorDecl(isUserProvided()) will match #1, but not #2 or #3.
+</pre></td></tr>
+
+
<tr><td>Matcher<<a href="http://clang.llvm.org/doxygen/classclang_1_1CXXConversionDecl.html";>CXXConversionDecl</a>></td><td class="name" onclick="toggle('isExplicit1')"><a name="isExplicit1Anchor">isExplicit</a></td><td></td></tr>
<tr><td colspan="4" class="doc" id="isExplicit1"><pre>Matches constructor and conversion declarations that are marked with
the explicit keyword.
@@ -2538,7 +2566,7 @@
</pre></td></tr>
-<tr><td>Matcher<<a href="http://clang.llvm.org/doxygen/classclang_1_1NamedDecl.html";>NamedDecl</a>></td><td class="name" onclick="toggle('hasName0')"><a name="hasName0Anchor">hasName</a></td><td>std::string Name</td></tr>
+<tr><td>Matcher<<a href="http://clang.llvm.org/doxygen/classclang_1_1NamedDecl.html";>NamedDecl</a>></td><td class="name" onclick="toggle('hasName0')"><a name="hasName0Anchor">hasName</a></td><td>std::string Name</td></tr>
<tr><td colspan="4" class="doc" id="hasName0"><pre>Matches NamedDecl nodes that have the specified name.
Supports specifying enclosing namespaces or classes by prefixing the name
@@ -2731,13 +2759,20 @@
<tr><td>Matcher<<a href="http://clang.llvm.org/doxygen/classclang_1_1QualType.html";>QualType</a>></td><td class="name" onclick="toggle('isAnyPointer0')"><a name="isAnyPointer0Anchor">isAnyPointer</a></td><td></td></tr>
-<tr><td colspan="4" class="doc" id="isAnyPointer0"><pre>Matches QualType nodes that are of any pointer type.
+<tr><td colspan="4" class="doc" id="isAnyPointer0"><pre>Matches QualType nodes that are of any pointer type; this includes
+the Objective-C object pointer type, which is different despite being
+syntactically similar.
Given
int *i = nullptr;
+
+ @interface Foo
+ @end
+ Foo *f;
+
int j;
varDecl(hasType(isAnyPointer()))
- matches "int *i", but not "int j".
+ matches "int *i" and "Foo *f", but not "int j".
</pre></td></tr>
@@ -3228,16 +3263,6 @@
</pre></td></tr>
-<tr><td>Matcher<internal::Matcher<<a href="http://clang.llvm.org/doxygen/classclang_1_1NamedDecl.html";>NamedDecl</a>>></td><td class="name" onclick="toggle('hasAnyName0')"><a name="hasAnyName0Anchor">hasAnyName</a></td><td>StringRef, ..., StringRef</td></tr>
-<tr><td colspan="4" class="doc" id="hasAnyName0"><pre>Matches NamedDecl nodes that have any of the specified names.
-
-This matcher is only provided as a performance optimization of hasName.
- hasAnyName(a, b, c)
- is equivalent but faster than
- anyOf(hasName(a), hasName(b), hasName(c))
-</pre></td></tr>
-
-
<tr><td>Matcher<internal::Matcher<<a href="http://clang.llvm.org/doxygen/classclang_1_1Stmt.html";>Stmt</a>>></td><td class="name" onclick="toggle('isInTemplateInstantiation0')"><a name="isInTemplateInstantiation0Anchor">isInTemplateInstantiation</a></td><td></td></tr>
<tr><td colspan="4" class="doc" id="isInTemplateInstantiation0"><pre>Matches statements inside of a template instantiation.
Index: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
===================================================================
--- test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
@@ -4,13 +4,13 @@
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{};
@@ -26,12 +26,12 @@
};
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{};
@@ -43,7 +43,7 @@
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;
@@ -67,7 +67,6 @@
};
NegativeFieldInitializedInDefinition::NegativeFieldInitializedInDefinition() : F() {}
-
struct NegativeInClassInitialized {
int F = 0;
@@ -87,25 +86,248 @@
};
#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{};
+};
Index: test/clang-tidy/cppcoreguidelines-pro-type-member-init-delayed.cpp
===================================================================
--- test/clang-tidy/cppcoreguidelines-pro-type-member-init-delayed.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-type-member-init-delayed.cpp
@@ -1,20 +1,20 @@
// 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;
@@ -25,7 +25,7 @@
// 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;
Index: test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp
===================================================================
--- test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp
@@ -3,13 +3,13 @@
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 */;
@@ -23,12 +23,12 @@
};
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;
@@ -40,7 +40,7 @@
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;
};
@@ -64,4 +64,32 @@
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;
+};
Index: docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
===================================================================
--- docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
+++ docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
@@ -3,16 +3,35 @@
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.
+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.
-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 takes assignment of fields in the constructor body into
+account but generates false positives for fields initialized in
+methods invoked in the constructor body.
-This rule is part of the "Type safety" profile of the C++ Core Guidelines, see
+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.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -270,6 +270,22 @@
* `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
--------------------------
Index: clang-tidy/utils/TypeTraits.h
===================================================================
--- clang-tidy/utils/TypeTraits.h
+++ clang-tidy/utils/TypeTraits.h
@@ -20,6 +20,13 @@
// \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
Index: clang-tidy/utils/TypeTraits.cpp
===================================================================
--- clang-tidy/utils/TypeTraits.cpp
+++ clang-tidy/utils/TypeTraits.cpp
@@ -31,6 +31,81 @@
!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
Index: clang-tidy/utils/Matchers.h
===================================================================
--- clang-tidy/utils/Matchers.h
+++ clang-tidy/utils/Matchers.h
@@ -23,6 +23,11 @@
return IsExpensive && *IsExpensive;
}
+AST_MATCHER(RecordDecl, isTriviallyDefaultConstructible) {
+ return type_traits::recordIsTriviallyDefaultConstructible(
+ Node, Finder->getASTContext());
+}
+
} // namespace matchers
} // namespace tidy
} // namespace clang
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
===================================================================
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
@@ -16,24 +16,53 @@
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
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -9,12 +9,15 @@
#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;
@@ -24,16 +27,13 @@
namespace {
-AST_MATCHER(CXXConstructorDecl, isUserProvided) {
- return Node.isUserProvided();
-}
-
-static void
-fieldsRequiringInit(const RecordDecl::field_range &Fields,
- SmallPtrSetImpl<const FieldDecl *> &FieldsToInit) {
+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);
}
}
@@ -50,185 +50,373 @@
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 << ", ";
- }
+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 Stream.str();
+ return llvm::join(Names.begin(), Names.end(), ", ");
}
-// Contains all fields in correct order that need to be inserted at the same
-// location for pre C++11.
-// There are 3 kinds of insertions:
-// 1. The fields are inserted after an existing CXXCtorInitializer stored in
-// InitializerBefore. This will be the case whenever there is a written
-// initializer before the fields available.
-// 2. The fields are inserted before the first existing initializer stored in
-// InitializerAfter.
-// 3. There are no written initializers and the fields will be inserted before
-// the constructor's body creating a new initializer list including the ':'.
-struct FieldsInsertion {
- const CXXCtorInitializer *InitializerBefore;
- const CXXCtorInitializer *InitializerAfter;
- SmallVector<const FieldDecl *, 4> Fields;
+SourceLocation 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
+ // 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
+ // 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 ':'.
+ 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() << "()";
+ 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;
}
- Stream.flush();
- // The initializer list is created, replace leading comma with colon.
- if (InitializerBefore == nullptr && InitializerAfter == nullptr)
- Code[1] = ':';
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;
- for (const CXXCtorInitializer *Init : Inits) {
- if (Init->isWritten() && !Init->isMemberInitializer())
- LastWrittenNonMemberInit = Init;
- }
- SmallVector<FieldsInsertion, 16> OrderedFields;
- OrderedFields.push_back({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);
- auto CurrentField = Fields.begin();
+ typename R::const_iterator Decl = std::begin(OrderedDecls);
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 (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));
+ }
}
- // 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, {}});
+
+ Insertions.emplace_back(InitializerPlacement::After, Init);
}
}
- // 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);
+
+ // 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))
+ .bind("ctor"),
+ this);
+ 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;
- FieldsToInit.erase(Init->getMember());
+ 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()),
"{}");
}
- return;
+ } else {
+ // Otherwise, rewrite the constructor's initializer list.
+ fixInitializerList(Context, Diag, Ctor, FieldsToFix);
}
- for (const auto &FieldsInsertion :
- computeInsertions(Ctor->inits(), MemberFields, FieldsToInit)) {
- if (!FieldsInsertion.Fields.empty())
- Diag << FixItHint::CreateInsertion(
- FieldsInsertion.getLocation(*Result.Context, *Ctor),
- FieldsInsertion.codeToInsert());
+}
+
+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());
+ }
+
+ 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
