[clang-tidy] Add a check for undelegated copy of base classes
Finds copy constructors where the constructor don't call
the copy constructor of the base class.
```
class X : public Copyable {
X(const X &other) {} // Copyable(other) is missing
};
```
Differential Revision: https://reviews.llvm.org/D33722
llvm-svn: 318522
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index c350c96..5b89a9e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "CopyConstructorInitCheck.h"
#include "IntegerDivisionCheck.h"
#include "SuspiciousMemsetUsageCheck.h"
#include "UndefinedMemoryManipulationCheck.h"
@@ -21,6 +22,8 @@
class BugproneModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<CopyConstructorInitCheck>(
+ "misc-copy-constructor-init");
CheckFactories.registerCheck<IntegerDivisionCheck>(
"bugprone-integer-division");
CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 0b83c96..91327da 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -2,6 +2,7 @@
add_clang_library(clangTidyBugproneModule
BugproneTidyModule.cpp
+ CopyConstructorInitCheck.cpp
IntegerDivisionCheck.cpp
SuspiciousMemsetUsageCheck.cpp
UndefinedMemoryManipulationCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/CopyConstructorInitCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CopyConstructorInitCheck.cpp
new file mode 100644
index 0000000..151e56c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/CopyConstructorInitCheck.cpp
@@ -0,0 +1,121 @@
+//===--- CopyConstructorInitCheck.cpp - clang-tidy-------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "CopyConstructorInitCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void CopyConstructorInitCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ // In the future this might be extended to move constructors?
+ Finder->addMatcher(
+ cxxConstructorDecl(
+ isCopyConstructor(),
+ hasAnyConstructorInitializer(cxxCtorInitializer(
+ isBaseInitializer(),
+ withInitializer(cxxConstructExpr(hasDeclaration(
+ cxxConstructorDecl(isDefaultConstructor())))))),
+ unless(isInstantiated()))
+ .bind("ctor"),
+ this);
+}
+
+void CopyConstructorInitCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
+ std::string ParamName = Ctor->getParamDecl(0)->getNameAsString();
+
+ // We want only one warning (and FixIt) for each ctor.
+ std::string FixItInitList;
+ bool HasRelevantBaseInit = false;
+ bool ShouldNotDoFixit = false;
+ bool HasWrittenInitializer = false;
+ SmallVector<FixItHint, 2> SafeFixIts;
+ for (const auto *Init : Ctor->inits()) {
+ bool CtorInitIsWritten = Init->isWritten();
+ HasWrittenInitializer = HasWrittenInitializer || CtorInitIsWritten;
+ if (!Init->isBaseInitializer())
+ continue;
+ const Type *BaseType = Init->getBaseClass();
+ // Do not do fixits if there is a type alias involved or one of the bases
+ // are explicitly initialized. In the latter case we not do fixits to avoid
+ // -Wreorder warnings.
+ if (const auto *TempSpecTy = dyn_cast<TemplateSpecializationType>(BaseType))
+ ShouldNotDoFixit = ShouldNotDoFixit || TempSpecTy->isTypeAlias();
+ ShouldNotDoFixit = ShouldNotDoFixit || isa<TypedefType>(BaseType);
+ ShouldNotDoFixit = ShouldNotDoFixit || CtorInitIsWritten;
+ const CXXRecordDecl *BaseClass =
+ BaseType->getAsCXXRecordDecl()->getDefinition();
+ if (BaseClass->field_empty() &&
+ BaseClass->forallBases(
+ [](const CXXRecordDecl *Class) { return Class->field_empty(); }))
+ continue;
+ bool NonCopyableBase = false;
+ for (const auto *Ctor : BaseClass->ctors()) {
+ if (Ctor->isCopyConstructor() &&
+ (Ctor->getAccess() == AS_private || Ctor->isDeleted())) {
+ NonCopyableBase = true;
+ break;
+ }
+ }
+ if (NonCopyableBase)
+ continue;
+ const auto *CExpr = dyn_cast<CXXConstructExpr>(Init->getInit());
+ if (!CExpr || !CExpr->getConstructor()->isDefaultConstructor())
+ continue;
+ HasRelevantBaseInit = true;
+ if (CtorInitIsWritten) {
+ if (!ParamName.empty())
+ SafeFixIts.push_back(
+ FixItHint::CreateInsertion(CExpr->getLocEnd(), ParamName));
+ } else {
+ if (Init->getSourceLocation().isMacroID() ||
+ Ctor->getLocation().isMacroID() || ShouldNotDoFixit)
+ break;
+ FixItInitList += BaseClass->getNameAsString();
+ FixItInitList += "(" + ParamName + "), ";
+ }
+ }
+ if (!HasRelevantBaseInit)
+ return;
+
+ auto Diag = diag(Ctor->getLocation(),
+ "calling a base constructor other than the copy constructor")
+ << SafeFixIts;
+
+ if (FixItInitList.empty() || ParamName.empty() || ShouldNotDoFixit)
+ return;
+
+ std::string FixItMsg{FixItInitList.substr(0, FixItInitList.size() - 2)};
+ SourceLocation FixItLoc;
+ // There is no initialization list in this constructor.
+ if (!HasWrittenInitializer) {
+ FixItLoc = Ctor->getBody()->getLocStart();
+ FixItMsg = " : " + FixItMsg;
+ } else {
+ // We apply the missing ctors at the beginning of the initialization list.
+ FixItLoc = (*Ctor->init_begin())->getSourceLocation();
+ FixItMsg += ',';
+ }
+ FixItMsg += ' ';
+
+ Diag << FixItHint::CreateInsertion(FixItLoc, FixItMsg);
+} // namespace misc
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/bugprone/CopyConstructorInitCheck.h b/clang-tools-extra/clang-tidy/bugprone/CopyConstructorInitCheck.h
new file mode 100644
index 0000000..4d13da4
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/CopyConstructorInitCheck.h
@@ -0,0 +1,36 @@
+//===--- CopyConstructorInitCheck.h - clang-tidy--------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_COPY_CONSTRUCTOR_INIT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_COPY_CONSTRUCTOR_INIT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds copy constructors where the ctor don't call the copy constructor of
+/// the base class.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-copy-constructor-init.html
+class CopyConstructorInitCheck : public ClangTidyCheck {
+public:
+ CopyConstructorInitCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_COPY_CONSTRUCTOR_INIT_H