malcolm.parsons created this revision.
malcolm.parsons added reviewers: alexfh, aaron.ballman, omtcyfz.
malcolm.parsons added subscribers: cfe-commits, mgehre.
Herald added a subscriber: nemanjai.
Handle classes with default constructors that are defaulted or are not
present in the AST.
Classes with virtual methods or virtual bases are not trivially
default constructible, so their members and bases need to be initialized.
https://reviews.llvm.org/D24965
Files:
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
clang-tidy/utils/TypeTraits.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
@@ -367,3 +367,25 @@
PositiveIndirectMember() {}
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A
};
+
+struct PositiveVirtualMethod {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: constructor does not initialize these fields: F
+ int F;
+ // CHECK-FIXES: int F{};
+ virtual int f() = 0;
+};
+
+struct PositiveVirtualDestructor {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: constructor does not initialize these fields: F
+ PositiveVirtualDestructor() = default;
+ int F;
+ // CHECK-FIXES: int F{};
+ virtual ~PositiveVirtualDestructor() {}
+};
+
+struct PositiveVirtualBase : public virtual NegativeAggregateType {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: constructor does not initialize these bases: NegativeAggregateType
+ // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: constructor does not initialize these fields: F
+ int F;
+ // CHECK-FIXES: int F{};
+};
Index: clang-tidy/utils/TypeTraits.cpp
===================================================================
--- clang-tidy/utils/TypeTraits.cpp
+++ clang-tidy/utils/TypeTraits.cpp
@@ -58,6 +58,9 @@
// constructible.
if (ClassDecl->hasUserProvidedDefaultConstructor())
return false;
+ // A polymorphic class is not trivially constructible
+ if (ClassDecl->isPolymorphic())
+ return false;
// A class is trivially constructible if it has a trivial default constructor.
if (ClassDecl->hasTrivialDefaultConstructor())
return true;
@@ -71,6 +74,8 @@
for (const CXXBaseSpecifier &Base : ClassDecl->bases()) {
if (!isTriviallyDefaultConstructible(Base.getType(), Context))
return false;
+ if (Base.isVirtual())
+ return false;
}
return true;
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
===================================================================
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
@@ -46,11 +46,13 @@
// To fix: Write a data member initializer, or mention it in the member
// initializer list.
void checkMissingMemberInitializer(ASTContext &Context,
+ const CXXRecordDecl *ClassDecl,
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 CXXRecordDecl *ClassDecl,
const CXXConstructorDecl *Ctor);
// Checks Type.6 part 2:
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -269,6 +269,19 @@
IsNonTrivialDefaultConstructor))
.bind("ctor"),
this);
+
+ // Match classes with a default constructor that is defaulted or is not in the
+ // AST.
+ Finder->addMatcher(
+ cxxRecordDecl(
+ isDefinition(),
+ anyOf(has(cxxConstructorDecl(isDefaultConstructor(), isDefaulted(),
+ unless(isImplicit()))),
+ unless(has(cxxConstructorDecl()))),
+ unless(isTriviallyDefaultConstructible()))
+ .bind("record"),
+ this);
+
auto HasDefaultConstructor = hasInitializer(
cxxConstructExpr(unless(requiresZeroInitialization()),
hasDeclaration(cxxConstructorDecl(
@@ -287,8 +300,13 @@
// Skip declarations delayed by late template parsing without a body.
if (!Ctor->getBody())
return;
- checkMissingMemberInitializer(*Result.Context, Ctor);
- checkMissingBaseClassInitializer(*Result.Context, Ctor);
+ checkMissingMemberInitializer(*Result.Context, Ctor->getParent(), Ctor);
+ checkMissingBaseClassInitializer(*Result.Context, Ctor->getParent(), Ctor);
+ } else if (const auto *Record =
+ Result.Nodes.getNodeAs<CXXRecordDecl>("record")) {
+ assert(Record->hasDefaultConstructor());
+ checkMissingMemberInitializer(*Result.Context, Record, nullptr);
+ checkMissingBaseClassInitializer(*Result.Context, Record, nullptr);
} else if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var")) {
checkUninitializedTrivialType(*Result.Context, Var);
}
@@ -299,8 +317,8 @@
}
void ProTypeMemberInitCheck::checkMissingMemberInitializer(
- ASTContext &Context, const CXXConstructorDecl *Ctor) {
- const CXXRecordDecl *ClassDecl = Ctor->getParent();
+ ASTContext &Context, const CXXRecordDecl *ClassDecl,
+ const CXXConstructorDecl *Ctor) {
bool IsUnion = ClassDecl->isUnion();
if (IsUnion && ClassDecl->hasInClassInitializer())
@@ -317,16 +335,18 @@
if (FieldsToInit.empty())
return;
- 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->getAnyMember());
+ if (Ctor) {
+ 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->getAnyMember());
+ }
}
+ removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit);
}
- removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit);
// Collect all fields in order, both direct fields and indirect fields from
// anonmyous record types.
@@ -342,14 +362,14 @@
return;
DiagnosticBuilder Diag =
- diag(Ctor->getLocStart(),
+ diag(Ctor ? Ctor->getLocStart() : ClassDecl->getLocation(),
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())
+ if (Ctor && Ctor->getLocStart().isMacroID())
return;
// Collect all fields but only suggest a fix for the first member of unions,
@@ -370,15 +390,15 @@
getLocationForEndOfToken(Context, Field->getSourceRange().getEnd()),
"{}");
}
- } else {
+ } else if (Ctor) {
// Otherwise, rewrite the constructor's initializer list.
fixInitializerList(Context, Diag, Ctor, FieldsToFix);
}
}
void ProTypeMemberInitCheck::checkMissingBaseClassInitializer(
- const ASTContext &Context, const CXXConstructorDecl *Ctor) {
- const CXXRecordDecl *ClassDecl = Ctor->getParent();
+ const ASTContext &Context, const CXXRecordDecl *ClassDecl,
+ const CXXConstructorDecl *Ctor) {
// Gather any base classes that need to be initialized.
SmallVector<const RecordDecl *, 4> AllBases;
@@ -397,20 +417,23 @@
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()->getAsCXXRecordDecl());
+ if (Ctor) {
+ for (const CXXCtorInitializer *Init : Ctor->inits()) {
+ if (Init->isBaseInitializer() && Init->isWritten())
+ BasesToInit.erase(Init->getBaseClass()->getAsCXXRecordDecl());
+ }
}
if (BasesToInit.empty())
return;
DiagnosticBuilder Diag =
- diag(Ctor->getLocStart(),
+ diag(Ctor ? Ctor->getLocStart() : ClassDecl->getLocation(),
"constructor does not initialize these bases: %0")
<< toCommaSeparatedString(AllBases, BasesToInit);
- fixInitializerList(Context, Diag, Ctor, BasesToInit);
+ if (Ctor)
+ fixInitializerList(Context, Diag, Ctor, BasesToInit);
}
void ProTypeMemberInitCheck::checkUninitializedTrivialType(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
