malcolm.parsons updated this revision to Diff 73019.
malcolm.parsons added a comment.
Rebase
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
@@ -377,3 +377,25 @@
{
NegativeInClassInitializedDefaulted s;
}
+
+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;
@@ -73,6 +76,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
