malcolm.parsons updated this revision to Diff 73474.
malcolm.parsons added a comment.
Pass records by reference.
Change test to trigger diagnostic for non-template type
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,49 @@
{
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{};
+};
+
+template <typename T>
+struct PositiveTemplateVirtualDestructor {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: constructor does not initialize these fields: F
+ T Val;
+ int F;
+ // CHECK-FIXES: int F{};
+ virtual ~PositiveTemplateVirtualDestructor() = default;
+};
+
+template struct PositiveTemplateVirtualDestructor<int>;
+
+#define UNINITIALIZED_FIELD_IN_MACRO_BODY_VIRTUAL(FIELD) \
+ struct UninitializedFieldVirtual##FIELD { \
+ int FIELD; \
+ virtual ~UninitializedFieldVirtual##FIELD() {} \
+ }; \
+// Ensure FIELD is not initialized since fixes inside of macros are disabled.
+// CHECK-FIXES: int FIELD;
+
+UNINITIALIZED_FIELD_IN_MACRO_BODY_VIRTUAL(F);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: F
+UNINITIALIZED_FIELD_IN_MACRO_BODY_VIRTUAL(G);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: G
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
@@ -32,17 +32,17 @@
// the record type (or indirect field) is a union, forEachField will stop after
// the first field.
template <typename T, typename Func>
-void forEachField(const RecordDecl *Record, const T &Fields,
+void forEachField(const RecordDecl &Record, const T &Fields,
bool OneFieldPerUnion, Func &&Fn) {
for (const FieldDecl *F : Fields) {
if (F->isAnonymousStructOrUnion()) {
if (const CXXRecordDecl *R = F->getType()->getAsCXXRecordDecl())
- forEachField(R, R->fields(), OneFieldPerUnion, Fn);
+ forEachField(*R, R->fields(), OneFieldPerUnion, Fn);
} else {
Fn(F);
}
- if (OneFieldPerUnion && Record->isUnion())
+ if (OneFieldPerUnion && Record.isUnion())
break;
}
}
@@ -214,16 +214,16 @@
// Gets the list of bases and members that could possibly be initialized, in
// order as they appear in the class declaration.
-void getInitializationsInOrder(const CXXRecordDecl *ClassDecl,
+void getInitializationsInOrder(const CXXRecordDecl &ClassDecl,
SmallVectorImpl<const NamedDecl *> &Decls) {
Decls.clear();
- for (const auto &Base : ClassDecl->bases()) {
+ for (const auto &Base : 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);
}
}
- forEachField(ClassDecl, ClassDecl->fields(), false,
+ forEachField(ClassDecl, ClassDecl.fields(), false,
[&](const FieldDecl *F) { Decls.push_back(F); });
}
@@ -236,7 +236,7 @@
return;
SmallVector<const NamedDecl *, 16> OrderedDecls;
- getInitializationsInOrder(Ctor->getParent(), OrderedDecls);
+ getInitializationsInOrder(*Ctor->getParent(), OrderedDecls);
for (const auto &Insertion :
computeInsertions(Ctor->inits(), OrderedDecls, DeclsToInit)) {
@@ -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(), unless(isInstantiated()),
+ 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,14 @@
// 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() &&
+ "Matched record should have a default constructor");
+ 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,39 +318,41 @@
}
void ProTypeMemberInitCheck::checkMissingMemberInitializer(
- ASTContext &Context, const CXXConstructorDecl *Ctor) {
- const CXXRecordDecl *ClassDecl = Ctor->getParent();
- bool IsUnion = ClassDecl->isUnion();
+ ASTContext &Context, const CXXRecordDecl &ClassDecl,
+ const CXXConstructorDecl *Ctor) {
+ bool IsUnion = ClassDecl.isUnion();
- if (IsUnion && ClassDecl->hasInClassInitializer())
+ if (IsUnion && ClassDecl.hasInClassInitializer())
return;
// Gather all fields (direct and indirect) that need to be initialized.
SmallPtrSet<const FieldDecl *, 16> FieldsToInit;
- forEachField(ClassDecl, ClassDecl->fields(), false, [&](const FieldDecl *F) {
+ 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;
- 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.
SmallVector<const FieldDecl *, 16> OrderedFields;
- forEachField(ClassDecl, ClassDecl->fields(), false,
+ forEachField(ClassDecl, ClassDecl.fields(), false,
[&](const FieldDecl *F) { OrderedFields.push_back(F); });
// Collect all the fields we need to initialize, including indirect fields.
@@ -342,14 +363,15 @@
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())
+ // Do not propose fixes for constructors in macros since we cannot place them
+ // correctly.
+ if (Ctor && Ctor->getLocStart().isMacroID())
return;
// Collect all fields but only suggest a fix for the first member of unions,
@@ -370,20 +392,20 @@
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;
SmallPtrSet<const RecordDecl *, 4> BasesToInit;
- for (const CXXBaseSpecifier &Base : ClassDecl->bases()) {
+ for (const CXXBaseSpecifier &Base : ClassDecl.bases()) {
if (const auto *BaseClassDecl = getCanonicalRecordDecl(Base.getType())) {
AllBases.emplace_back(BaseClassDecl);
if (!BaseClassDecl->field_empty() &&
@@ -397,20 +419,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
