[clang-tidy] Add a checker for code that looks like a delegate constructors but doesn't delegate.

Summary:
class Foo {
  Foo() {
    Foo(42); // oops
  }
  Foo(int);
};

This is valid code but it does nothing and we can't emit a warning in clang
because there might be side effects. The checker emits a warning for this
pattern and also for base class initializers written in this style.

There is some overlap with the unused-rtti checker but they follow different
goals and fire in different places most of the time.

Reviewers: alexfh, djasper

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D4667

llvm-svn: 214397

clang-tools-extra/clang-tidy/misc/CMakeLists.txt

@@ -6,6 +6,7 @@ add_clang_library(clangTidyMiscModule
   MiscTidyModule.cpp
   RedundantSmartptrGet.cpp
   SwappedArgumentsCheck.cpp
+  UndelegatedConstructor.cpp
   UnusedRAII.cpp
   UseOverride.cpp
 

clang-tools-extra/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"
 
@@ -35,6 +36,9 @@ public:
     CheckFactories.addCheckFactory(
         "misc-swapped-arguments",
         new ClangTidyCheckFactory<SwappedArgumentsCheck>());
+    CheckFactories.addCheckFactory(
+        "misc-undelegated-constructor",
+        new ClangTidyCheckFactory<UndelegatedConstructorCheck>());
     CheckFactories.addCheckFactory(
         "misc-unused-raii",
         new ClangTidyCheckFactory<UnusedRAIICheck>());

clang-tools-extra/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<Stmt>,
+              InnerMatcher) {
+  const Stmt *E = &Node;
+  for (;;) {
+    // Temporaries with non-trivial dtors.
+    if (const auto *EWC = dyn_cast<ExprWithCleanups>(E))
+      E = EWC->getSubExpr();
+    // Temporaries with zero or more than two ctor arguments.
+    else if (const auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
+      E = BTE->getSubExpr();
+    // Temporaries with exactly one ctor argument.
+    else if (const auto *FCE = dyn_cast<CXXFunctionalCastExpr>(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<CXXRecordDecl>(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<CXXConstructExpr>("construct");
+  diag(E->getLocStart(), "did you intend to call a delegated constructor? "
+                         "A temporary object is created here instead");
+}
+
+} // namespace tidy
+} // namespace clang

clang-tools-extra/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

clang-tools-extra/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 <typename T>
+struct TDerived : public Base {
+  TDerived() { Base(); }
+};
+
+TDerived<int> t;