Index: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt @@ -6,6 +6,7 @@ MiscTidyModule.cpp RedundantSmartptrGet.cpp SwappedArgumentsCheck.cpp + UndelegatedConstructor.cpp UnusedRAII.cpp UseOverride.cpp Index: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp @@ -14,6 +14,7 @@ #include "BoolPointerImplicitConversion.h" #include "RedundantSmartptrGet.h" #include "SwappedArgumentsCheck.h" +#include "UndelegatedConstructor.h" #include "UnusedRAII.h" #include "UseOverride.h" @@ -36,6 +37,9 @@ "misc-swapped-arguments", new ClangTidyCheckFactory()); CheckFactories.addCheckFactory( + "misc-undelegated-constructor", + new ClangTidyCheckFactory()); + CheckFactories.addCheckFactory( "misc-unused-raii", new ClangTidyCheckFactory()); CheckFactories.addCheckFactory( Index: clang-tools-extra/trunk/clang-tidy/misc/UndelegatedConstructor.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/UndelegatedConstructor.h +++ clang-tools-extra/trunk/clang-tidy/misc/UndelegatedConstructor.h @@ -0,0 +1,30 @@ +//===--- UndelegatedConstructor.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_MISC_UNDELEGATED_CONSTRUCTOR_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNDELEGATED_CONSTRUCTOR_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// \brief Finds creation of temporary objects in constructors that look like a +/// function call to another constructor of the same class. The user most likely +/// meant to use a delegating constructor or base class initializer. +class UndelegatedConstructorCheck : public ClangTidyCheck { +public: + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNDELEGATED_CONSTRUCTOR_H Index: clang-tools-extra/trunk/clang-tidy/misc/UndelegatedConstructor.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/UndelegatedConstructor.cpp +++ clang-tools-extra/trunk/clang-tidy/misc/UndelegatedConstructor.cpp @@ -0,0 +1,76 @@ +//===--- UndelegatedConstructor.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 "UndelegatedConstructor.h" +#include "clang/AST/ASTContext.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { + +namespace ast_matchers { +AST_MATCHER_P(Stmt, ignoringTemporaryExpr, internal::Matcher, + InnerMatcher) { + const Stmt *E = &Node; + for (;;) { + // Temporaries with non-trivial dtors. + if (const auto *EWC = dyn_cast(E)) + E = EWC->getSubExpr(); + // Temporaries with zero or more than two ctor arguments. + else if (const auto *BTE = dyn_cast(E)) + E = BTE->getSubExpr(); + // Temporaries with exactly one ctor argument. + else if (const auto *FCE = dyn_cast(E)) + E = FCE->getSubExpr(); + else + break; + } + + return InnerMatcher.matches(*E, Finder, Builder); +} + +// Finds a node if it's a base of an already bound node. +AST_MATCHER_P(CXXRecordDecl, baseOfBoundNode, std::string, ID) { + return Builder->removeBindings([&](const internal::BoundNodesMap &Nodes) { + const auto *Derived = Nodes.getNodeAs(ID); + return Derived != &Node && !Derived->isDerivedFrom(&Node); + }); +} +} // namespace ast_matchers + +namespace tidy { + +void UndelegatedConstructorCheck::registerMatchers(MatchFinder *Finder) { + // We look for calls to constructors of the same type in constructors. To do + // this we have to look through a variety of nodes that occur in the path, + // depending on the type's destructor and the number of arguments on the + // constructor call, this is handled by ignoringTemporaryExpr. Ignore template + // instantiations to reduce the number of duplicated warnings. + Finder->addMatcher( + compoundStmt( + hasParent(constructorDecl(ofClass(recordDecl().bind("parent")))), + forEach(ignoringTemporaryExpr( + constructExpr(hasDeclaration(constructorDecl(ofClass( + recordDecl(baseOfBoundNode("parent")))))) + .bind("construct"))), + unless(hasAncestor(decl( + anyOf(recordDecl(ast_matchers::isTemplateInstantiation()), + functionDecl(ast_matchers::isTemplateInstantiation())))))), + this); +} + +void UndelegatedConstructorCheck::check(const MatchFinder::MatchResult &Result) { + const auto *E = Result.Nodes.getStmtAs("construct"); + diag(E->getLocStart(), "did you intend to call a delegated constructor? " + "A temporary object is created here instead"); +} + +} // namespace tidy +} // namespace clang Index: clang-tools-extra/trunk/test/clang-tidy/misc-undelegated-constructor.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/misc-undelegated-constructor.cpp +++ clang-tools-extra/trunk/test/clang-tidy/misc-undelegated-constructor.cpp @@ -0,0 +1,54 @@ +// RUN: clang-tidy -checks='-*,misc-undelegated-constructor' %s -- -std=c++11 2>&1 | FileCheck %s -implicit-check-not='{{warning:|error:}}' + +struct Ctor; +Ctor foo(); + +struct Ctor { + Ctor(); + Ctor(int); + Ctor(int, int); + Ctor(Ctor *i) { + Ctor(); +// CHECK: :[[@LINE-1]]:5: warning: did you intend to call a delegated constructor? A temporary object is created here instead + Ctor(0); +// CHECK: :[[@LINE-1]]:5: warning: did you intend to call a delegated constructor? A temporary object is created here instead + Ctor(1, 2); +// CHECK: :[[@LINE-1]]:5: warning: did you intend to call a delegated constructor? A temporary object is created here instead + foo(); + } +}; + +Ctor::Ctor() { + Ctor(1); +// CHECK: :[[@LINE-1]]:3: warning: did you intend to call a delegated constructor? A temporary object is created here instead +} + +Ctor::Ctor(int i) : Ctor(i, 1) {} // properly delegated. + +struct Dtor { + Dtor(); + Dtor(int); + Dtor(int, int); + Dtor(Ctor *i) { + Dtor(); +// CHECK: :[[@LINE-1]]:5: warning: did you intend to call a delegated constructor? A temporary object is created here instead + Dtor(0); +// CHECK: :[[@LINE-1]]:5: warning: did you intend to call a delegated constructor? A temporary object is created here instead + Dtor(1, 2); +// CHECK: :[[@LINE-1]]:5: warning: did you intend to call a delegated constructor? A temporary object is created here instead + } + ~Dtor(); +}; + +struct Base {}; +struct Derived : public Base { + Derived() { Base(); } +// CHECK: :[[@LINE-1]]:15: warning: did you intend to call a delegated constructor? A temporary object is created here instead +}; + +template +struct TDerived : public Base { + TDerived() { Base(); } +}; + +TDerived t;