sbarzowski updated this revision to Diff 99502.
sbarzowski marked 8 inline comments as done.
sbarzowski added a comment.

Removed unnecessary colon from message


https://reviews.llvm.org/D19201

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/ThrowWithNoexceptCheck.cpp
  clang-tidy/misc/ThrowWithNoexceptCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-throw-with-noexcept.rst
  test/clang-tidy/misc-throw-with-noexcept.cpp

Index: test/clang-tidy/misc-throw-with-noexcept.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-throw-with-noexcept.cpp
@@ -0,0 +1,201 @@
+// RUN: %check_clang_tidy %s misc-throw-with-noexcept %t
+
+void f_throw_with_ne() noexcept(true) {
+  {
+    throw 5;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-6]]:24: note: in a function declared nothrow here
+// CHECK-FIXES: void f_throw_with_ne() {
+
+void f_noexcept_false() noexcept(false) {
+  throw 5;
+}
+
+void f_decl_only() noexcept;
+
+
+void throw_and_catch() noexcept(true) {
+  try {
+    throw 5;
+  } catch (...) {
+  }
+}
+
+struct A{};
+struct B{};
+
+void throw_and_catch_something_else() noexcept(true) {
+  try {
+    throw A();
+  } catch (B b) {
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-7]]:39: note: in a function declared nothrow here
+// CHECK-FIXES: void throw_and_catch_something_else() {
+
+
+void throw_and_catch_the_same_thing() noexcept {
+  try {
+    throw A();
+  } catch (A a) {
+  }
+}
+
+
+void throw_and_catch_int() noexcept {
+  try {
+    throw 42;
+  } catch (int a) {
+  }
+}
+
+
+void throw_and_catch_() noexcept {
+  try {
+    throw 42;
+  } catch (int a) {
+  }
+}
+
+
+void throw_and_catch_pointer() noexcept {
+  try {
+    throw A();
+  } catch (A *a) {
+  }
+}
+
+class Base{};
+class Derived: public Base {};
+
+void throw_and_catch_base_class() noexcept {
+  try {
+    throw Derived();
+  } catch (Base &x) {
+  }
+}
+
+void throw_and_catch_nested() noexcept {
+  try {
+    try {
+        throw Derived();
+    } catch (int x) {
+    }
+  } catch (Base &x) {
+  }
+}
+
+void throw_and_catch_derived_class() noexcept {
+  try {
+    throw Base();
+  } catch (Derived &x) {
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-7]]:38: note: in a function declared nothrow here
+// CHECK-FIXES: void throw_and_catch_derived_class() {
+
+
+class Class {
+  void InClass() const noexcept(true) {
+    throw 42;
+  }
+};
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-5]]:24: note: in a function declared nothrow here
+// CHECK-FIXES: void InClass() const {
+
+
+void forward_declared() noexcept;
+
+void forward_declared() noexcept {
+	throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:2: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:25: note: in a function declared nothrow here
+// CHECK-MESSAGES: :[[@LINE-7]]:25: note: in a function declared nothrow here
+// CHECK-FIXES: void forward_declared() ;
+// CHECK-FIXES: void forward_declared() {
+
+void getFunction() noexcept {
+  struct X {
+    static void inner()
+    {
+        throw 42;
+    }
+  };
+}
+
+void dynamic_exception_spec() throw() {
+  throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: in a function declared nothrow here
+// CHECK-FIXES: void dynamic_exception_spec() {
+
+#define NOEXCEPT noexcept
+
+void with_macro() NOEXCEPT {
+  throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: in a function declared nothrow here
+// CHECK-FIXES: void with_macro() {
+
+template<typename T> int template_function() noexcept {
+  throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:46: note: in a function declared nothrow here
+// CHECK-FIXES: template<typename T> int template_function() {
+
+template<typename T> class template_class {
+  int throw_in_member() noexcept {
+    throw 42;
+  }
+  // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+  // CHECK-MESSAGES: :[[@LINE-4]]:25: note: in a function declared nothrow here
+  // CHECK-FIXES: int throw_in_member() {
+};
+
+void instantiations() {
+  template_function<int>();
+  template_function<float>();
+  template_class<int> c1;
+  template_class<float> c2;
+}
+
+struct throw_from_destructor {
+  ~throw_from_destructor() {
+    throw 42;
+  }
+  // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: note: in a destructor defined here
+};
+
+struct throw_from_throwing_destructor {
+  ~throw_from_throwing_destructor() noexcept(false) {
+    throw 42;
+  }
+};
+
+void with_parens() noexcept((true)) {
+  throw 42;
+}
+
+struct magic_compile_time_computation {
+  static constexpr bool result = true;
+};
+
+void noexcept_based_on_computation() noexcept(magic_compile_time_computation::result) {
+  throw 42;
+}
+
+void sometimes_based_on_computation() noexcept(magic_compile_time_computation::result);
+
+void sometimes_based_on_computation() noexcept(true) {
+  throw 42;
+}
Index: docs/clang-tidy/checks/misc-throw-with-noexcept.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-throw-with-noexcept.rst
@@ -0,0 +1,30 @@
+.. title:: clang-tidy - misc-throw-with-noexcept
+
+misc-throw-with-noexcept
+========================
+
+This check finds cases of using ``throw`` in a function declared
+with a non-throwing exception specification.
+
+The warning is *not* issued when the exception is caught in the same function.
+
+It removes the exception specification as a fix.
+
+
+  .. code-block:: c++
+
+    void f() noexcept {
+    	throw 42;
+    }
+
+    // Will be changed to
+    void f() {
+    	throw 42;
+    }
+
+    void f() noexcept {
+      try {
+        throw 42; // perfectly ok
+      } catch(int x) {
+      }
+    }
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -107,6 +107,7 @@
    misc-suspicious-string-compare
    misc-swapped-arguments
    misc-throw-by-value-catch-by-reference
+   misc-throw-with-noexcept
    misc-unconventional-assign-operator
    misc-undelegated-constructor
    misc-uniqueptr-reset-release
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,11 +52,14 @@
 Improvements to clang-rename
 ----------------------------
 
-The improvements are...
-
 Improvements to clang-tidy
 --------------------------
 
+- New `misc-throw-with-noexcept
+  <http://clang.llvm.org/extra/clang-tidy/checks/misc-throw-with-noexcept.html>`_ check
+
+  Flags ``throw`` statements in functions marked as no-throw.
+
 - New `cert-dcl21-cpp
   <http://clang.llvm.org/extra/clang-tidy/checks/cert-dcl21-cpp.html>`_ check
 
@@ -71,7 +74,7 @@
   <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-no-malloc.html>`_ check
 
   Allow custom memory management functions to be considered as well.
-  
+
 - New `misc-forwarding-reference-overload
   <http://clang.llvm.org/extra/clang-tidy/checks/misc-forwarding-reference-overload.html>`_ check
 
@@ -87,7 +90,7 @@
 
   Finds and replaces explicit calls to the constructor in a return statement by
   a braced initializer list so that the return type is not needlessly repeated.
-  
+
 - Improved `modernize-use-emplace
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html>`_ check
 
@@ -101,7 +104,7 @@
 
   Finds possible inefficient vector operations in for loops that may cause
   unnecessary memory reallocations.
-  
+
 - Added `ParameterThreshold` to `readability-function-size
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-function-size.html>`_ check
 
@@ -111,10 +114,10 @@
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-misleading-indentation.html>`_ check
 
   Finds misleading indentation where braces should be introduced or the code should be reformatted.
-  
+
 - Support clang-formatting of the code around applied fixes (``-format-style``
   command-line option).
-  
+
 - New `hicpp` module
 
   Adds checks that implement the `High Integrity C++ Coding Standard <http://www.codingstandard.com/section/index/>`_ and other safety
Index: clang-tidy/misc/ThrowWithNoexceptCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/ThrowWithNoexceptCheck.h
@@ -0,0 +1,33 @@
+//===--- ThrowWithNoexceptCheck.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_THROW_WITH_NOEXCEPT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_THROW_WITH_NOEXCEPT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+///\brief Warns about using throw in function declared as noexcept.
+/// It doesn't warn if the exception in caught in the same function.
+class ThrowWithNoexceptCheck : public ClangTidyCheck {
+public:
+  ThrowWithNoexceptCheck(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_THROW_WITH_NOEXCEPT_H
Index: clang-tidy/misc/ThrowWithNoexceptCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/ThrowWithNoexceptCheck.cpp
@@ -0,0 +1,136 @@
+//===--- ThrowWithNoexceptCheck.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 "ThrowWithNoexceptCheck.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 misc {
+
+void ThrowWithNoexceptCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+  Finder->addMatcher(
+      cxxThrowExpr(stmt(forFunction(functionDecl(isNoThrow()).bind("func"))))
+          .bind("throw"),
+      this);
+}
+
+bool isSimpleConstant(const Expr *E) { return isa<CXXBoolLiteralExpr>(E); }
+
+bool isCaughtWithType(const Type *Caught, const Type *Thrown) {
+  // FIXME: The logic below is a very rough approximation of the real rules.
+  // The real rules are much more complicated and probably
+  // should be implemented elsewhere.
+
+  if (Caught == nullptr) // the case of catch(...)
+    return true;
+
+  assert(Thrown != nullptr);
+  if (Caught == Thrown)
+    return true;
+
+  const auto *CaughtAsRecordType = Caught->getPointeeCXXRecordDecl();
+  const auto *ThrownAsRecordType = Thrown->getAsCXXRecordDecl();
+
+  if (CaughtAsRecordType && ThrownAsRecordType) {
+    if (CaughtAsRecordType == ThrownAsRecordType)
+      return true;
+    return ThrownAsRecordType->isDerivedFrom(CaughtAsRecordType);
+  }
+
+  return false;
+}
+
+bool isCaughtInTry(const CXXThrowExpr *Throw, const CXXTryStmt *Try) {
+  for (unsigned int i = 0; i < Try->getNumHandlers(); ++i) {
+    const auto *Catch = Try->getHandler(i);
+    const auto *CaughtType = Catch->getCaughtType().getTypePtrOrNull();
+    const auto *ThrownType = Throw->getSubExpr()->getType().getTypePtrOrNull();
+    return isCaughtWithType(CaughtType, ThrownType);
+  }
+  return false;
+}
+
+bool isCaughtInFunction(ASTContext *Context, const CXXThrowExpr *Throw,
+                         const FunctionDecl *Function,
+                         const ast_type_traits::DynTypedNode node) {
+  if (node.get<FunctionDecl>() == Function)
+    return false;
+
+  const auto *Try = node.get<CXXTryStmt>();
+  if (Try != nullptr && isCaughtInTry(Throw, Try))
+    return true;
+
+  for (const auto Parent : Context->getParents(node)) {
+    if (isCaughtInFunction(Context, Throw, Function, Parent))
+      return true;
+  }
+
+  return false;
+}
+
+void ThrowWithNoexceptCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("func");
+  const auto *Throw = Result.Nodes.getNodeAs<CXXThrowExpr>("throw");
+
+  llvm::SmallVector<SourceRange, 5> NoExceptRanges;
+  for (const auto *Redecl : Function->redecls()) {
+    const auto *Proto = Redecl->getType()->getAs<FunctionProtoType>();
+
+    const auto *NoExceptExpr = Proto->getNoexceptExpr();
+    if (NoExceptExpr != nullptr && !isSimpleConstant(NoExceptExpr)) {
+      // There is a complex noexcept expression, so we assume that
+      // it can potentially be true or false, based on the compilation
+      // flags etc.
+      return;
+    }
+
+    SourceRange NoExceptRange = Redecl->getExceptionSpecSourceRange();
+
+    if (NoExceptRange.isValid()) {
+      NoExceptRanges.push_back(NoExceptRange);
+    } else {
+      // If a single one is not valid, we cannot apply the fix as we need to
+      // remove noexcept in all declarations for the fix to be effective.
+      NoExceptRanges.clear();
+      break;
+    }
+  }
+
+  const auto ThrowNode = ast_type_traits::DynTypedNode::create(*Throw);
+  if (isCaughtInFunction(Result.Context, Throw, Function, ThrowNode)) {
+    return;
+  }
+
+  diag(Throw->getLocStart(), "'throw' expression in a function declared with a "
+                             "non-throwing exception specification");
+
+  const auto *Destructor = Result.Nodes.getNodeAs<CXXDestructorDecl>("func");
+  if (Destructor != nullptr) {
+    diag(Destructor->getSourceRange().getBegin(),
+         "in a destructor defined here", DiagnosticIDs::Note);
+    return;
+  }
+
+  for (const auto &NoExceptRange : NoExceptRanges) {
+    diag(NoExceptRange.getBegin(),
+         "in a function declared nothrow here", DiagnosticIDs::Note)
+        << FixItHint::CreateRemoval(NoExceptRange);
+  }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -47,6 +47,7 @@
 #include "SwappedArgumentsCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UnconventionalAssignOperatorCheck.h"
+#include "ThrowWithNoexceptCheck.h"
 #include "UndelegatedConstructor.h"
 #include "UniqueptrResetReleaseCheck.h"
 #include "UnusedAliasDeclsCheck.h"
@@ -129,6 +130,8 @@
         "misc-swapped-arguments");
     CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
         "misc-throw-by-value-catch-by-reference");
+    CheckFactories.registerCheck<ThrowWithNoexceptCheck>(
+        "misc-throw-with-noexcept");
     CheckFactories.registerCheck<UndelegatedConstructorCheck>(
         "misc-undelegated-constructor");
     CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -39,6 +39,7 @@
   SuspiciousStringCompareCheck.cpp
   SwappedArgumentsCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp
+  ThrowWithNoexceptCheck.cpp
   UndelegatedConstructor.cpp
   UniqueptrResetReleaseCheck.cpp
   UnusedAliasDeclsCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to