[clang-tidy] New checker for exceptions that are created but not thrown

Patch by: Kristof Umann

Differential Revision: https://reviews.llvm.org/D43120

llvm-svn: 325222

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -25,6 +25,7 @@
 #include "MultipleStatementMacroCheck.h"
 #include "StringConstructorCheck.h"
 #include "SuspiciousMemsetUsageCheck.h"
+#include "ThrowKeywordMissingCheck.h"
 #include "UndefinedMemoryManipulationCheck.h"
 #include "UseAfterMoveCheck.h"
 #include "VirtualNearMissCheck.h"
@@ -66,6 +67,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-string-constructor");
     CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(
         "bugprone-suspicious-memset-usage");
+    CheckFactories.registerCheck<ThrowKeywordMissingCheck>(
+        "bugprone-throw-keyword-missing");
     CheckFactories.registerCheck<UndefinedMemoryManipulationCheck>(
         "bugprone-undefined-memory-manipulation");
     CheckFactories.registerCheck<UseAfterMoveCheck>(

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

@@ -17,6 +17,7 @@ add_clang_library(clangTidyBugproneModule
   MultipleStatementMacroCheck.cpp
   StringConstructorCheck.cpp
   SuspiciousMemsetUsageCheck.cpp
+  ThrowKeywordMissingCheck.cpp
   UndefinedMemoryManipulationCheck.cpp
   UseAfterMoveCheck.cpp
   VirtualNearMissCheck.cpp

clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp

@@ -0,0 +1,52 @@
+//===--- ThrowKeywordMissingCheck.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 "ThrowKeywordMissingCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void ThrowKeywordMissingCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  auto CtorInitializerList =
+      cxxConstructorDecl(hasAnyConstructorInitializer(anything()));
+
+  Finder->addMatcher(
+      expr(anyOf(cxxFunctionalCastExpr(), cxxBindTemporaryExpr(),
+                 cxxTemporaryObjectExpr()),
+           hasType(cxxRecordDecl(
+               isSameOrDerivedFrom(matchesName("[Ee]xception|EXCEPTION")))),
+           unless(anyOf(hasAncestor(stmt(
+                            anyOf(cxxThrowExpr(), callExpr(), returnStmt()))),
+                        hasAncestor(varDecl()),
+                        allOf(hasAncestor(CtorInitializerList),
+                              unless(hasAncestor(cxxCatchStmt()))))))
+          .bind("temporary-exception-not-thrown"),
+      this); 
+}
+
+void ThrowKeywordMissingCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *TemporaryExpr =
+      Result.Nodes.getNodeAs<Expr>("temporary-exception-not-thrown");
+
+  diag(TemporaryExpr->getLocStart(), "suspicious exception object created but "
+                                     "not thrown; did you mean 'throw %0'?")
+      << TemporaryExpr->getType().getBaseTypeIdentifier()->getName();
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.h

@@ -0,0 +1,36 @@
+//===--- ThrowKeywordMissingCheck.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_THROWKEYWORDMISSINGCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_THROWKEYWORDMISSINGCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Emits a warning about temporary objects whose type is (or is derived from) a
+/// class that has 'EXCEPTION', 'Exception' or 'exception' in its name.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-throw-keyword-missing.html
+class ThrowKeywordMissingCheck : public ClangTidyCheck {
+public:
+  ThrowKeywordMissingCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_THROWKEYWORDMISSINGCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -94,6 +94,11 @@ Improvements to clang-tidy
   `cppcoreguidelines-avoid-goto <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-goto.html>`_
   added.
 
+- New `bugprone-throw-keyword-missing
+  <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-throw-keyword-missing.html>`_ check
+
+  Diagnoses when a temporary object that appears to be an exception is constructed but not thrown.
+
 Improvements to include-fixer
 -----------------------------
 

clang-tools-extra/docs/clang-tidy/checks/bugprone-throw-keyword-missing.rst

@@ -0,0 +1,20 @@
+.. title:: clang-tidy - bugprone-throw-keyword-missing
+
+bugprone-throw-keyword-missing
+==============================
+
+Warns about a potentially missing ``throw`` keyword. If a temporary object is created, but the
+object's type derives from (or is the same as) a class that has 'EXCEPTION', 'Exception' or
+'exception' in its name, we can assume that the programmer's intention was to throw that object.
+
+Example:
+
+.. code-block:: c++
+  void f(int i) {
+    if (i < 0) {
+      // Exception is created but is not thrown.
+      std::runtime_error("Unexpected argument");
+    }
+  }
+
+

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -32,6 +32,7 @@ Clang-Tidy Checks
    bugprone-multiple-statement-macro
    bugprone-string-constructor
    bugprone-suspicious-memset-usage
+   bugprone-throw-keyword-missing
    bugprone-undefined-memory-manipulation
    bugprone-use-after-move
    bugprone-virtual-near-miss

clang-tools-extra/test/clang-tidy/bugprone-throw-keyword-missing.cpp

@@ -0,0 +1,167 @@
+// RUN: %check_clang_tidy %s bugprone-throw-keyword-missing %t
+
+namespace std {
+
+// std::string declaration (taken from test/clang-tidy/readability-redundant-string-cstr-msvc.cpp).
+template <typename T>
+class allocator {};
+template <typename T>
+class char_traits {};
+template <typename C, typename T = std::char_traits<C>, typename A = std::allocator<C>>
+struct basic_string {
+  basic_string();
+  basic_string(const basic_string &);
+  // MSVC headers define two constructors instead of using optional arguments.
+  basic_string(const C *);
+  basic_string(const C *, const A &);
+  ~basic_string();
+};
+typedef basic_string<char> string;
+typedef basic_string<wchar_t> wstring;
+
+// std::exception and std::runtime_error declaration.
+struct exception {
+  exception();
+  exception(const exception &other);
+  virtual ~exception();
+};
+
+struct runtime_error : public exception {
+  explicit runtime_error(const std::string &what_arg);
+};
+
+} // namespace std
+
+// The usage of this class should never emit a warning.
+struct RegularClass {};
+
+// Class name contains the substring "exception", in certain cases using this class should emit a warning.
+struct RegularException {
+  RegularException() {}
+
+  // Constructors with a single argument are treated differently (cxxFunctionalCastExpr).
+  RegularException(int) {}
+};
+
+// --------------
+
+void stdExceptionNotTrownTest(int i) {
+  if (i < 0)
+    // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception object created but not thrown; did you mean 'throw {{.*}}'? [bugprone-throw-keyword-missing]
+    std::exception();
+
+  if (i > 0)
+    // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception
+    std::runtime_error("Unexpected argument");
+}
+
+void stdExceptionThrownTest(int i) {
+  if (i < 0)
+    throw std::exception();
+
+  if (i > 0)
+    throw std::runtime_error("Unexpected argument");
+}
+
+void regularClassNotThrownTest(int i) {
+  if (i < 0)
+    RegularClass();
+}
+
+void regularClassThrownTest(int i) {
+  if (i < 0)
+    throw RegularClass();
+}
+
+void nameContainsExceptionNotThrownTest(int i) {
+  if (i < 0)
+    // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception
+    RegularException();
+
+  if (i > 0)
+    // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception
+    RegularException(5);
+}
+
+void nameContainsExceptionThrownTest(int i) {
+  if (i < 0)
+    throw RegularException();
+
+  if (i > 0)
+    throw RegularException(5);
+}
+
+template <class Exception>
+void f(int i, Exception excToBeThrown) {}
+
+void funcCallWithTempExcTest() {
+  f(5, RegularException());
+}
+
+// Global variable initilization test.
+RegularException exc = RegularException();
+RegularException *excptr = new RegularException();
+
+void localVariableInitTest() {
+  RegularException exc = RegularException();
+  RegularException *excptr = new RegularException();
+}
+
+class CtorInitializerListTest {
+  RegularException exc;
+
+  CtorInitializerListTest() : exc(RegularException()) {}
+
+  CtorInitializerListTest(int) try : exc(RegularException()) {
+    // Constructor body
+  } catch (...) {
+    // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception
+    RegularException();
+  }
+
+  CtorInitializerListTest(float);
+};
+
+CtorInitializerListTest::CtorInitializerListTest(float) try : exc(RegularException()) {
+  // Constructor body
+} catch (...) {
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: suspicious exception
+  RegularException();
+}
+
+RegularException funcReturningExceptionTest(int i) {
+  return RegularException();
+}
+
+void returnedValueTest() {
+  funcReturningExceptionTest(3);
+}
+
+struct ClassBracedInitListTest {
+  ClassBracedInitListTest(RegularException exc) {}
+};
+
+void foo(RegularException, ClassBracedInitListTest) {}
+
+void bracedInitListTest() {
+  RegularException exc{};
+  ClassBracedInitListTest test = {RegularException()};
+  foo({}, {RegularException()});
+}
+
+typedef std::exception ERROR_BASE;
+class RegularError : public ERROR_BASE {};
+
+void typedefTest() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: suspicious exception
+  RegularError();
+}
+
+struct ExceptionRAII {
+  ExceptionRAII() {}
+  ~ExceptionRAII() {}
+};
+
+void exceptionRAIITest() {
+  ExceptionRAII E;
+}