Szelethus updated this revision to Diff 133860.
Szelethus added a comment.

Fixed almost everything mentioned in comments.

I also came up with this problem:

   RegularException funcReturningExceptioniTest(int i) {
     return RegularException();
   }
   
   void returnedValueTest() {
     funcReturningExceptioniTest(3); //Should this emit a warning?
  }

I'm not sure whether it'd be a good idea to warn about these cases. Unused 
return values can be found by many other means, and I'm afraid the standard 
library is filled with these cases.

I've also added this code snippet to the test file.


https://reviews.llvm.org/D43120

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/ThrowKeywordMissingCheck.cpp
  clang-tidy/misc/ThrowKeywordMissingCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-throw-keyword-missing.rst
  test/clang-tidy/misc-throw-keyword-missing.cpp

Index: test/clang-tidy/misc-throw-keyword-missing.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-throw-keyword-missing.cpp
@@ -0,0 +1,167 @@
+// RUN: %check_clang_tidy %s misc-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 {};
+
+// Classname 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 {{.*}}'? [misc-throw-keyword-missing]
+    std::exception();
+
+  if (i > 0)
+    //CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception object created but not thrown; did you mean 'throw {{.*}}'? [misc-throw-keyword-missing]
+    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 object created but not thrown; did you mean 'throw {{.*}}'? [misc-throw-keyword-missing]
+    RegularException();
+
+  if (i > 0)
+    //CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception object created but not thrown; did you mean 'throw {{.*}}'? [misc-throw-keyword-missing]
+    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 object created but not thrown; did you mean 'throw {{.*}}'? [misc-throw-keyword-missing]
+    RegularException();
+  }
+
+  CtorInitializerListTest(float);
+};
+
+CtorInitializerListTest::CtorInitializerListTest(float) try : exc(RegularException()) {
+  //Constructor body
+} catch (...) {
+  //CHECK-MESSAGES: :[[@LINE+1]]:3: warning: suspicious exception object created but not thrown; did you mean 'throw {{.*}}'? [misc-throw-keyword-missing]
+  RegularException();
+}
+
+RegularException funcReturningExceptioniTest(int i) {
+  return RegularException();
+}
+
+void returnedValueTest() {
+  funcReturningExceptioniTest(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 object created but not thrown; did you mean 'throw {{.*}}'? [misc-throw-keyword-missing]
+  RegularError();
+}
+
+struct ExceptionRAII {
+  ExceptionRAII() {}
+  ~ExceptionRAII() {}
+};
+
+void exceptionRAIITest() {
+  ExceptionRAII E;
+}
Index: docs/clang-tidy/checks/misc-throw-keyword-missing.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-throw-keyword-missing.rst
@@ -0,0 +1,20 @@
+.. title:: clang-tidy - misc-throw-keyword-missing
+
+misc-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");
+    }
+  }
+
+
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -143,6 +143,7 @@
    misc-suspicious-string-compare
    misc-swapped-arguments
    misc-throw-by-value-catch-by-reference
+   misc-throw-keyword-missing
    misc-unconventional-assign-operator
    misc-undelegated-constructor
    misc-uniqueptr-reset-release
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -64,6 +64,11 @@
   object is statically initialized with a ``constexpr`` constructor or has no 
   explicit constructor.
 
+- New `misc-throw-keyword-missing
+  <http://clang.llvm.org/extra/clang-tidy/checks/misc-throw-keyword-missing.html>`_ check
+
+  Warns if a ``throw`` keyword is potentialy missing before a temporary exception object.
+
 Improvements to include-fixer
 -----------------------------
 
Index: clang-tidy/misc/ThrowKeywordMissingCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/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_MISC_THROWKEYWORDMISSINGCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_THROWKEYWORDMISSINGCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// 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/misc-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 misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_THROWKEYWORDMISSINGCHECK_H
Index: clang-tidy/misc/ThrowKeywordMissingCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/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 misc {
+
+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 misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -33,6 +33,7 @@
 #include "SuspiciousStringCompareCheck.h"
 #include "SwappedArgumentsCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
+#include "ThrowKeywordMissingCheck.h"
 #include "UnconventionalAssignOperatorCheck.h"
 #include "UndelegatedConstructor.h"
 #include "UniqueptrResetReleaseCheck.h"
@@ -53,6 +54,8 @@
     CheckFactories.registerCheck<LambdaFunctionNameCheck>(
         "misc-lambda-function-name");
     CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
+    CheckFactories.registerCheck<ThrowKeywordMissingCheck>(
+        "misc-throw-keyword-missing");
     CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
         "misc-unconventional-assign-operator");
     CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -4,6 +4,7 @@
   ForwardingReferenceOverloadCheck.cpp
   LambdaFunctionNameCheck.cpp
   MisplacedConstCheck.cpp
+  ThrowKeywordMissingCheck.cpp
   UnconventionalAssignOperatorCheck.cpp
   DefinitionsInHeadersCheck.cpp
   IncorrectRoundings.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to