Re: [PATCH] D19993: Fixed cppcoreguidelines-pro-type-member-init when checking records with indirect fields
michael_miller updated this revision to Diff 56439.
michael_miller added a comment.
Switched to using getAsCXXRecordDecl.
http://reviews.llvm.org/D19993
Files:
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp
test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
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
@@ -357,3 +357,13 @@
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NegativeAggregateType
// CHECK-FIXES: PositiveSelfInitialization() : NegativeAggregateType(), PositiveSelfInitialization() {}
};
+
+class PositiveIndirectMember {
+ struct {
+int *A;
+// CHECK-FIXES: int *A{};
+ };
+
+ PositiveIndirectMember() {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A
+};
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
@@ -103,3 +103,14 @@
int X;
};
+
+class PositiveIndirectMember {
+ struct {
+int *A;
+ };
+
+ PositiveIndirectMember() : A() {}
+ PositiveIndirectMember(int) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A
+ // CHECK-FIXES: PositiveIndirectMember(int) : A() {}
+};
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -27,16 +27,23 @@
namespace {
-void fieldsRequiringInit(const RecordDecl::field_range ,
- ASTContext ,
- SmallPtrSetImpl ) {
+// Iterate over all the fields in a record type, both direct and indirect (e.g.
+// if the record contains an anonmyous struct). If OneFieldPerUnion is true and
+// the record type (or indirect field) is a union, forEachField will stop after
+// the first field.
+template
+void forEachField(const RecordDecl *Record, const T ,
+ bool OneFieldPerUnion, Func &) {
for (const FieldDecl *F : Fields) {
-if (F->hasInClassInitializer())
- continue;
-QualType Type = F->getType();
-if (!F->hasInClassInitializer() &&
-utils::type_traits::isTriviallyDefaultConstructible(Type, Context))
- FieldsToInit.insert(F);
+if (F->isAnonymousStructOrUnion()) {
+ if (const CXXRecordDecl *R = F->getType()->getAsCXXRecordDecl())
+forEachField(R, R->fields(), OneFieldPerUnion, Fn);
+} else {
+ Fn(F);
+}
+
+if (OneFieldPerUnion && Record->isUnion())
+ break;
}
}
@@ -179,8 +186,8 @@
// Gets either the field or base class being initialized by the provided
// initializer.
const auto *InitDecl =
- Init->isMemberInitializer()
- ? static_cast(Init->getMember())
+ Init->isAnyMemberInitializer()
+ ? static_cast(Init->getAnyMember())
: Init->getBaseClass()->getAsCXXRecordDecl();
// Add all fields between current field up until the next intializer.
@@ -216,7 +223,8 @@
Decls.emplace_back(Decl);
}
}
- Decls.append(ClassDecl->fields().begin(), ClassDecl->fields().end());
+ forEachField(ClassDecl, ClassDecl->fields(), false,
+ [&](const FieldDecl *F) { Decls.push_back(F); });
}
template
@@ -238,22 +246,6 @@
}
}
-template
-void forEachField(const RecordDecl *Record, const T ,
- bool OneFieldPerUnion, Func &) {
- 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;
- }
-}
-
} // anonymous namespace
ProTypeMemberInitCheck::ProTypeMemberInitCheck(StringRef Name,
@@ -314,8 +306,14 @@
if (IsUnion && ClassDecl->hasInClassInitializer())
return;
+ // Gather all fields (direct and indirect) that need to be initialized.
SmallPtrSet FieldsToInit;
- fieldsRequiringInit(ClassDecl->fields(), Context, FieldsToInit);
+ forEachField(ClassDecl, ClassDecl->fields(), false, [&](const FieldDecl *F) {
+if (!F->hasInClassInitializer() &&
+utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
+Context))
+ FieldsToInit.insert(F);
+ });
if (FieldsToInit.empty())
return;
@@ -325,7
Re: [PATCH] D19993: Fixed cppcoreguidelines-pro-type-member-init when checking records with indirect fields
michael_miller added inline comments.
Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:31
@@ +30,3 @@
+// Convenience utility to get a RecordDecl from a QualType.
+const RecordDecl *getCanonicalRecordDecl(const QualType ) {
+ if (const auto *RT = Type.getCanonicalType()->getAs())
alexfh wrote:
> Is `getCanonicalType()` important here? Did you try using
> `QualType::getAsCXXRecordDecl()`?
Probably not but I didn't try it. I just moved the previous function up to the
top so I didn't have to forward declare it.
One thing I'm simply unsure of is whether it's possible to get a RecordDecl
that's not a CXXRecordDecl in C++ code. It seems like the answer is no but that
might be another reason to keep it as is if I'm wrong about that...
http://reviews.llvm.org/D19993
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19993: Fixed cppcoreguidelines-pro-type-member-init when checking records with indirect fields
michael_miller created this revision.
michael_miller added reviewers: hokein, aaron.ballman, alexfh.
michael_miller added a subscriber: cfe-commits.
Fixed a crash in cppcoreguidelines-pro-type-member-init when checking record
types with indirect fields pre-C++11.
Fixed handling of indirect fields so they are properly checked and suggested
fixes are proposed.
http://reviews.llvm.org/D19993
Files:
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp
test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
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
@@ -357,3 +357,13 @@
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NegativeAggregateType
// CHECK-FIXES: PositiveSelfInitialization() : NegativeAggregateType(), PositiveSelfInitialization() {}
};
+
+class PositiveIndirectMember {
+ struct {
+int *A;
+// CHECK-FIXES: int *A{};
+ };
+
+ PositiveIndirectMember() {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A
+};
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
@@ -103,3 +103,14 @@
int X;
};
+
+class PositiveIndirectMember {
+ struct {
+int *A;
+ };
+
+ PositiveIndirectMember() : A() {}
+ PositiveIndirectMember(int) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A
+ // CHECK-FIXES: PositiveIndirectMember(int) : A() {}
+};
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -27,16 +27,30 @@
namespace {
-void fieldsRequiringInit(const RecordDecl::field_range ,
- ASTContext ,
- SmallPtrSetImpl ) {
+// Convenience utility to get a RecordDecl from a QualType.
+const RecordDecl *getCanonicalRecordDecl(const QualType ) {
+ if (const auto *RT = Type.getCanonicalType()->getAs())
+return RT->getDecl();
+ return nullptr;
+}
+
+// Iterate over all the fields in a record type, both direct and indirect (e.g.
+// if the record contains an anonmyous struct). If OneFieldPerUnion is true and
+// the record type (or indirect field) is a union, forEachField will stop after
+// the first field.
+template
+void forEachField(const RecordDecl *Record, const T ,
+ bool OneFieldPerUnion, Func &) {
for (const FieldDecl *F : Fields) {
-if (F->hasInClassInitializer())
- continue;
-QualType Type = F->getType();
-if (!F->hasInClassInitializer() &&
-utils::type_traits::isTriviallyDefaultConstructible(Type, Context))
- FieldsToInit.insert(F);
+if (F->isAnonymousStructOrUnion()) {
+ if (const RecordDecl *R = getCanonicalRecordDecl(F->getType()))
+forEachField(R, R->fields(), OneFieldPerUnion, Fn);
+} else {
+ Fn(F);
+}
+
+if (OneFieldPerUnion && Record->isUnion())
+ break;
}
}
@@ -155,13 +169,6 @@
SmallVector Initializers;
};
-// Convenience utility to get a RecordDecl from a QualType.
-const RecordDecl *getCanonicalRecordDecl(const QualType ) {
- if (const auto *RT = Type.getCanonicalType()->getAs())
-return RT->getDecl();
- return nullptr;
-}
-
template
SmallVector
computeInsertions(const CXXConstructorDecl::init_const_range ,
@@ -179,8 +186,8 @@
// Gets either the field or base class being initialized by the provided
// initializer.
const auto *InitDecl =
- Init->isMemberInitializer()
- ? static_cast(Init->getMember())
+ Init->isAnyMemberInitializer()
+ ? static_cast(Init->getAnyMember())
: Init->getBaseClass()->getAsCXXRecordDecl();
// Add all fields between current field up until the next intializer.
@@ -216,7 +223,8 @@
Decls.emplace_back(Decl);
}
}
- Decls.append(ClassDecl->fields().begin(), ClassDecl->fields().end());
+ forEachField(ClassDecl, ClassDecl->fields(), false,
+ [&](const FieldDecl *F) { Decls.push_back(F); });
}
template
@@ -238,22 +246,6 @@
}
}
-template
-void forEachField(const RecordDecl *Record, const T ,
- bool OneFieldPerUnion, Func &) {
- for (const FieldDecl *F : Fields) {
-if (F->isAnonymousStructOrUnion()) {
- if (const RecordDecl *R =
Re: [PATCH] D19802: Fix a crash in cppcoreguidelines-pro-type-member-init when checking a class that initializes itself as a base
michael_miller updated this revision to Diff 55904.
michael_miller added a comment.
Added a comment explaining the new test added.
http://reviews.llvm.org/D19802
Files:
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
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
@@ -338,3 +338,14 @@
Template F;
};
+
+// This pathological template fails to compile if actually instantiated. It
+// results in the check seeing a null RecordDecl when examining the base class
+// initializer list.
+template
+class PositiveSelfInitialization : NegativeAggregateType
+{
+ PositiveSelfInitialization() : PositiveSelfInitialization() {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize
these bases: NegativeAggregateType
+ // CHECK-FIXES: PositiveSelfInitialization() : NegativeAggregateType(),
PositiveSelfInitialization() {}
+};
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -179,7 +179,7 @@
const auto *InitDecl =
Init->isMemberInitializer()
? static_cast(Init->getMember())
- : Init->getBaseClass()->getAs()->getDecl();
+ : Init->getBaseClass()->getAsCXXRecordDecl();
// Add all fields between current field up until the next intializer.
for (; Decl != std::end(OrderedDecls) && *Decl != InitDecl; ++Decl) {
@@ -398,7 +398,7 @@
// 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()->getDecl());
+ BasesToInit.erase(Init->getBaseClass()->getAsCXXRecordDecl());
}
if (BasesToInit.empty())
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
@@ -338,3 +338,14 @@
Template F;
};
+
+// This pathological template fails to compile if actually instantiated. It
+// results in the check seeing a null RecordDecl when examining the base class
+// initializer list.
+template
+class PositiveSelfInitialization : NegativeAggregateType
+{
+ PositiveSelfInitialization() : PositiveSelfInitialization() {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NegativeAggregateType
+ // CHECK-FIXES: PositiveSelfInitialization() : NegativeAggregateType(), PositiveSelfInitialization() {}
+};
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -179,7 +179,7 @@
const auto *InitDecl =
Init->isMemberInitializer()
? static_cast(Init->getMember())
- : Init->getBaseClass()->getAs()->getDecl();
+ : Init->getBaseClass()->getAsCXXRecordDecl();
// Add all fields between current field up until the next intializer.
for (; Decl != std::end(OrderedDecls) && *Decl != InitDecl; ++Decl) {
@@ -398,7 +398,7 @@
// 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()->getDecl());
+ BasesToInit.erase(Init->getBaseClass()->getAsCXXRecordDecl());
}
if (BasesToInit.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19802: Fix a crash in cppcoreguidelines-pro-type-member-init when checking a class that initializes itself as a base
michael_miller marked 4 inline comments as done. Comment at: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp:342 @@ +341,3 @@ + +// This pathological template fails to compile if actually instantiated. It +// results in the check seeing a null RecordDecl when examining the base class Added a comment explaining why the check is there. http://reviews.llvm.org/D19802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19802: Fix a crash in cppcoreguidelines-pro-type-member-init when checking a class that initializes itself as a base
michael_miller added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp:342 @@ +341,3 @@ + +template +class PositiveSelfInitialization : NegativeAggregateType aaron.ballman wrote: > hokein wrote: > > aaron.ballman wrote: > > > Is it required to be a templated class to trigger the crash? > > Yes, it's required. > Then some comments in this test would be good, I would assume otherwise from > just looking at the test. It's a little bit of a pathological example. Without the template, the code won't compile at all due to an error. The check doesn't crash in that case (error: constructor for 'PositiveSelfInitialization' creates a delegation cycle [clang-diagnostic-delegating-ctor-cycles]). With the template in the mix, apparently PositiveSelfInitialization doesn't have a valid RecordDecl. If you instantiate the template, though, the code again fails to compile. http://reviews.llvm.org/D19802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19802: Fix a crash in cppcoreguidelines-pro-type-member-init when checking a class that initializes itself as a base
michael_miller created this revision.
michael_miller added reviewers: hokein, alexfh, aaron.ballman.
michael_miller added a subscriber: cfe-commits.
Fix a crash when a record type initializes itself in its own base class
initializer list.
http://reviews.llvm.org/D19802
Files:
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
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
@@ -338,3 +338,11 @@
Template F;
};
+
+template
+class PositiveSelfInitialization : NegativeAggregateType
+{
+ PositiveSelfInitialization() : PositiveSelfInitialization() {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize
these bases: NegativeAggregateType
+ // CHECK-FIXES: PositiveSelfInitialization() : NegativeAggregateType(),
PositiveSelfInitialization() {}
+};
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -179,7 +179,7 @@
const auto *InitDecl =
Init->isMemberInitializer()
? static_cast(Init->getMember())
- : Init->getBaseClass()->getAs()->getDecl();
+ : Init->getBaseClass()->getAsCXXRecordDecl();
// Add all fields between current field up until the next intializer.
for (; Decl != std::end(OrderedDecls) && *Decl != InitDecl; ++Decl) {
@@ -398,7 +398,7 @@
// 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()->getDecl());
+ BasesToInit.erase(Init->getBaseClass()->getAsCXXRecordDecl());
}
if (BasesToInit.empty())
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
@@ -338,3 +338,11 @@
Template F;
};
+
+template
+class PositiveSelfInitialization : NegativeAggregateType
+{
+ PositiveSelfInitialization() : PositiveSelfInitialization() {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NegativeAggregateType
+ // CHECK-FIXES: PositiveSelfInitialization() : NegativeAggregateType(), PositiveSelfInitialization() {}
+};
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -179,7 +179,7 @@
const auto *InitDecl =
Init->isMemberInitializer()
? static_cast(Init->getMember())
- : Init->getBaseClass()->getAs()->getDecl();
+ : Init->getBaseClass()->getAsCXXRecordDecl();
// Add all fields between current field up until the next intializer.
for (; Decl != std::end(OrderedDecls) && *Decl != InitDecl; ++Decl) {
@@ -398,7 +398,7 @@
// 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()->getDecl());
+ BasesToInit.erase(Init->getBaseClass()->getAsCXXRecordDecl());
}
if (BasesToInit.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19672: [clang-tidy] cppcoreguidelines-pro-type-member-init should not complain about static variables
michael_miller added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:284 @@ -283,2 +283,3 @@ varDecl(isDefinition(), HasDefaultConstructor, + hasAutomaticStorageDuration(), hasType(recordDecl(has(fieldDecl()), alexfh wrote: > aaron.ballman wrote: > > We should add a test that this still diagnoses variables with dynamic > > storage duration. Alternatively, this could be > > `unless(anyOf(hasStaticStorageDuration(), hasThreadStorageDuration()))` > I don't think it ever diagnosed on the dynamic storage duration, since it > doesn't match `cxxNewExpr` ;) Perhaps unrelated but a new expression without the parens wouldn't value-initialize a record type with a default constructor and potentially be a problematic. Maybe we should be checking cxxNewExpr. http://reviews.llvm.org/D19672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19539: Fix a crash in cppcoreguidelines-pro-type-member-init when checking a type with a template parameter as a base class.
michael_miller created this revision.
michael_miller added reviewers: hokein, aaron.ballman, alexfh.
michael_miller added a subscriber: cfe-commits.
Fixed a crash in cppcoreguidelines-pro-type-member-init when encountering a
type that uses one of its template parameters as a base when compiling for
C++98.
http://reviews.llvm.org/D19539
Files:
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp
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
@@ -70,26 +70,36 @@
int Z;
};
-struct NonTrivialType {
+struct TrivialType {
int X;
int Y;
};
struct PositiveUninitializedBaseOrdering : public NegativeAggregateType,
- public NonTrivialType {
- PositiveUninitializedBaseOrdering() : NegativeAggregateType(),
NonTrivialType(), B() {}
+ public TrivialType {
+ PositiveUninitializedBaseOrdering() : NegativeAggregateType(),
TrivialType(), B() {}
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize
these fields: A
- // CHECK-FIXES: PositiveUninitializedBaseOrdering() :
NegativeAggregateType(), NonTrivialType(), A(), B() {}
+ // CHECK-FIXES: PositiveUninitializedBaseOrdering() :
NegativeAggregateType(), TrivialType(), A(), B() {}
// This is somewhat pathological with the base class initializer at the
end...
- PositiveUninitializedBaseOrdering(int) : B(), NonTrivialType(), A() {}
+ PositiveUninitializedBaseOrdering(int) : B(), TrivialType(), A() {}
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize
these bases: NegativeAggregateType
- // CHECK-FIXES: PositiveUninitializedBaseOrdering(int) : B(),
NegativeAggregateType(), NonTrivialType(), A() {}
+ // CHECK-FIXES: PositiveUninitializedBaseOrdering(int) : B(),
NegativeAggregateType(), TrivialType(), A() {}
PositiveUninitializedBaseOrdering(float) : NegativeAggregateType(), A() {}
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize
these bases: NonTrivialType
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize
these bases: TrivialType
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: constructor does not initialize
these fields: B
- // CHECK-FIXES: PositiveUninitializedBaseOrdering(float) :
NegativeAggregateType(), NonTrivialType(), A(), B() {}
+ // CHECK-FIXES: PositiveUninitializedBaseOrdering(float) :
NegativeAggregateType(), TrivialType(), A(), B() {}
int A, B;
};
+
+template
+class PositiveTemplateBase : T {
+public:
+ PositiveTemplateBase() {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize
these fields: X
+ // CHECK-FIXES: PositiveTemplateBase() : X() {}
+
+ int X;
+};
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -208,8 +208,12 @@
void getInitializationsInOrder(const CXXRecordDecl *ClassDecl,
SmallVectorImpl ) {
Decls.clear();
- for (const auto : ClassDecl->bases())
-Decls.emplace_back(getCanonicalRecordDecl(Base.getType()));
+ for (const auto : ClassDecl->bases()) {
+// Decl may be null if the base class is a template parameter.
+if (const NamedDecl *Decl = getCanonicalRecordDecl(Base.getType())) {
+ Decls.emplace_back(Decl);
+}
+ }
Decls.append(ClassDecl->fields().begin(), ClassDecl->fields().end());
}
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
@@ -70,26 +70,36 @@
int Z;
};
-struct NonTrivialType {
+struct TrivialType {
int X;
int Y;
};
struct PositiveUninitializedBaseOrdering : public NegativeAggregateType,
- public NonTrivialType {
- PositiveUninitializedBaseOrdering() : NegativeAggregateType(), NonTrivialType(), B() {}
+ public TrivialType {
+ PositiveUninitializedBaseOrdering() : NegativeAggregateType(), TrivialType(), B() {}
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A
- // CHECK-FIXES: PositiveUninitializedBaseOrdering() : NegativeAggregateType(), NonTrivialType(), A(), B() {}
+ // CHECK-FIXES: PositiveUninitializedBaseOrdering() : NegativeAggregateType(), TrivialType(), A(), B() {}
// This is somewhat pathological with the base
Re: [PATCH] D19270: Fix a crash in cppcoreguidelines-pro-type-member-init related to missing constructor bodies.
michael_miller added a comment. In http://reviews.llvm.org/D19270#405805, @alexfh wrote: > LG. Do you need me to submit the patch? Yeah, that's be awesome—thanks! http://reviews.llvm.org/D19270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller added a comment. In http://reviews.llvm.org/D18584#404901, @hokein wrote: > In http://reviews.llvm.org/D18584#404192, @michael_miller wrote: > > > In http://reviews.llvm.org/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 (http://reviews.llvm.org/D19270). Repository: rL LLVM http://reviews.llvm.org/D18584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19270: Fix a crash in cppcoreguidelines-pro-type-member-init related to missing constructor bodies.
michael_miller created this revision.
michael_miller added reviewers: alexfh, aaron.ballman, hokein.
michael_miller added a subscriber: cfe-commits.
Fixes a crash in cppcoreguidelines-pro-type-member-init when checking some
record types with a constructor without a body. We now check to make sure the
constructor has a body before looking for missing members and base initializers.
http://reviews.llvm.org/D19270
Files:
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
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
@@ -331,3 +331,10 @@
int X;
// CHECK-FIXES: int X{};
};
+
+// This check results in a CXXConstructorDecl with no body.
+struct NegativeDeletedConstructor : NegativeAggregateType {
+ NegativeDeletedConstructor() = delete;
+
+ Template F;
+};
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -285,6 +285,9 @@
void ProTypeMemberInitCheck::check(const MatchFinder::MatchResult ) {
if (const auto *Ctor = Result.Nodes.getNodeAs("ctor")) {
+// Skip declarations delayed by late template parsing without a body.
+if (!Ctor->getBody())
+ return;
checkMissingMemberInitializer(*Result.Context, Ctor);
checkMissingBaseClassInitializer(*Result.Context, Ctor);
} else if (const auto *Var = Result.Nodes.getNodeAs("var")) {
@@ -304,11 +307,6 @@
if (IsUnion && ClassDecl->hasInClassInitializer())
return;
- // Skip declarations delayed by late template parsing without a body.
- const Stmt *Body = Ctor->getBody();
- if (!Body)
-return;
-
SmallPtrSet FieldsToInit;
fieldsRequiringInit(ClassDecl->fields(), Context, FieldsToInit);
if (FieldsToInit.empty())
@@ -323,7 +321,7 @@
FieldsToInit.erase(Init->getMember());
}
}
- removeFieldsInitializedInBody(*Body, Context, FieldsToInit);
+ removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit);
// Collect all fields in order, both direct fields and indirect fields from
// anonmyous record types.
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
@@ -331,3 +331,10 @@
int X;
// CHECK-FIXES: int X{};
};
+
+// This check results in a CXXConstructorDecl with no body.
+struct NegativeDeletedConstructor : NegativeAggregateType {
+ NegativeDeletedConstructor() = delete;
+
+ Template F;
+};
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -285,6 +285,9 @@
void ProTypeMemberInitCheck::check(const MatchFinder::MatchResult ) {
if (const auto *Ctor = Result.Nodes.getNodeAs("ctor")) {
+// Skip declarations delayed by late template parsing without a body.
+if (!Ctor->getBody())
+ return;
checkMissingMemberInitializer(*Result.Context, Ctor);
checkMissingBaseClassInitializer(*Result.Context, Ctor);
} else if (const auto *Var = Result.Nodes.getNodeAs("var")) {
@@ -304,11 +307,6 @@
if (IsUnion && ClassDecl->hasInClassInitializer())
return;
- // Skip declarations delayed by late template parsing without a body.
- const Stmt *Body = Ctor->getBody();
- if (!Body)
-return;
-
SmallPtrSet FieldsToInit;
fieldsRequiringInit(ClassDecl->fields(), Context, FieldsToInit);
if (FieldsToInit.empty())
@@ -323,7 +321,7 @@
FieldsToInit.erase(Init->getMember());
}
}
- removeFieldsInitializedInBody(*Body, Context, FieldsToInit);
+ removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit);
// Collect all fields in order, both direct fields and indirect fields from
// anonmyous record types.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller added a comment. In http://reviews.llvm.org/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. Repository: rL LLVM http://reviews.llvm.org/D18584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller updated this revision to Diff 53480.
michael_miller added a comment.
Broke out the cfe parts into its own change (http://reviews.llvm.org/D19038).
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/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
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
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
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
[PATCH] D19038: Add AST Matchers for CXXConstructorDecl::isDelegatingConstructor and CXXMethodDecl::isUserProvided.
michael_miller created this revision.
michael_miller added reviewers: alexfh, aaron.ballman.
michael_miller added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
Added two AST matchers: isDelegatingConstructor for
CXXConstructorDecl::IsDelegatingConstructor; and isUserProvided corresponding
to CXXMethodDecl::isUserProvided.
http://reviews.llvm.org/D19038
Files:
docs/LibASTMatchersReference.html
include/clang/ASTMatchers/ASTMatchers.h
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
@@ -3803,6 +3803,21 @@
return Node.size_overridden_methods() > 0 || Node.hasAttr();
}
+/// \brief Matches method 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(CXXMethodDecl, isUserProvided) {
+ return Node.isUserProvided();
+}
+
/// \brief Matches member expressions that are called with '->' as opposed
/// to '.'.
///
@@ -4911,6 +4926,23 @@
return Node.isDefaultConstructor();
}
+/// \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 @@
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html;>CXXConstructorDeclisDelegatingConstructor
+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.
+
+
+
Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html;>CXXConstructorDeclisExplicit
Matches constructor and conversion declarations that are marked with
the explicit keyword.
@@ -1983,6 +1998,19 @@
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html;>CXXMethodDeclisUserProvided
+Matches method 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.
+
+
+
Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html;>CXXMethodDeclisVirtual
Matches if the given method declaration is
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller added a comment. In http://reviews.llvm.org/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? http://reviews.llvm.org/D18584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller added a comment. In http://reviews.llvm.org/D18584#398929, @alexfh wrote: > Do you have svn commit access? I do not... this is my first contribution to llvm/clang. http://reviews.llvm.org/D18584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller marked an inline comment as done. michael_miller added a comment. In http://reviews.llvm.org/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? http://reviews.llvm.org/D18584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller added a comment. In http://reviews.llvm.org/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. http://reviews.llvm.org/D18584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
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 @@
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html;>CXXConstructorDeclisDelegatingConstructor
+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.
+
+
+
Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html;>CXXConstructorDeclisExplicit
Matches constructor and conversion declarations that are marked with
the explicit keyword.
@@ -1828,6 +1843,19 @@
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html;>CXXConstructorDeclisUserProvided
+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.
+
+
+
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller marked 3 inline comments as done. michael_miller added a comment. Sounds good! Updated with suggested changes. http://reviews.llvm.org/D18584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller updated this revision to Diff 53380.
michael_miller marked 2 inline comments as done.
michael_miller added a comment.
Herald added a subscriber: klimek.
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.
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,37 @@
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(DestructorDeclaration, MatchesVirtualDestructor) {
+ EXPECT_TRUE(matches("class Foo { virtual ~Foo(); };",
+ cxxDestructorDecl(ofClass(hasName("Foo");
+}
+
+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 @@
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html;>CXXConstructorDeclisDelegatingConstructor
+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.
+
+
+
Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html;>CXXConstructorDeclisExplicit
Matches constructor and conversion
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller marked 4 inline comments as done.
michael_miller added a comment.
In http://reviews.llvm.org/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!
Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:25
@@ -24,3 +24,3 @@
-namespace {
aaron.ballman wrote:
> Please do not remove the unnamed namespace; it is preferable to using static
> functions.
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
Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:51
@@ +50,3 @@
+ return true;
+}
+
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.
Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:243
@@ +242,3 @@
+// initializer.
+static const NamedDecl *getInitializerDecl(const CXXCtorInitializer *Init) {
+ if (Init->isMemberInitializer())
aaron.ballman wrote:
> Since this function is only used once, I think it should be lowered into its
> use as a ternary conditional.
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.
Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:486
@@ +485,3 @@
+
+ // Do not propose fixes in macros since we cannot place them correctly.
+ if (Ctor->getLocStart().isMacroID())
aaron.ballman wrote:
> I think this code belongs in `fixInitializerList` then, doesn't it?
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.
Comment at: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp:334
@@ +333,2 @@
+ // CHECK-FIXES: int X{};
+};
aaron.ballman wrote:
> 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;
> };
> ```
For a number of reasons, it doesn't hit any warnings:
# We don't do anything to record types without any members.
# U doesn't have a user-provided constructor. We only initialize members if
there's a user-provided constructor of some sort.
# 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.
http://reviews.llvm.org/D18584
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller updated this revision to Diff 52950.
michael_miller marked 15 inline comments as done.
michael_miller added a comment.
Removed a superfluous cast and added periods to some comments.
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/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
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
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
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:
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller updated this revision to Diff 52238.
michael_miller added a comment.
Ran clang-format on all modified source files.
http://reviews.llvm.org/D18584
Files:
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
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
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
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,238 @@
};
#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
Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller updated this revision to Diff 52156.
michael_miller added a comment.
Added explanation of changes to ReleaseNotes.rst and cleaned up documentation
for the check a little.
http://reviews.llvm.org/D18584
Files:
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
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
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
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;
@@ -95,17 +95,226 @@
// 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 \
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;
+};
+
[PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.
michael_miller created this revision.
michael_miller added reviewers: flx, alexfh.
michael_miller added a subscriber: cfe-commits.
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.
http://reviews.llvm.org/D18584
Files:
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
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
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
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;
@@ -95,17 +95,226 @@
// 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 \
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
