sbarzowski updated this revision to Diff 97859.
sbarzowski added a comment.
Herald added a subscriber: xazax.hun.
Fixed false positive issues
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,191 @@
+// 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 no-throw 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 no-throw 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_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 no-throw 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 no-throw 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 no-throw here:
+// CHECK-MESSAGES: :[[@LINE-7]]:25: note: in a function declared no-throw 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 no-throw 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 no-throw 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 no-throw 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 no-throw 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,24 @@
+.. 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.
+
+Please note that the warning is issued even if the exception is caught within
+the same function, as that would be probably a bad style anyway.
+
+It removes the exception specification as a fix.
+
+
+ .. code-block:: c++
+
+ void f() noexcept {
+ throw 42;
+ }
+
+ // Will be changed to
+ void f() {
+ throw 42;
+ }
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -106,6 +106,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-dcl58-cpp
<http://clang.llvm.org/extra/clang-tidy/checks/cert-dcl58-cpp.html>`_ check
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 complains about every throw, even if it is caught later.
+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,138 @@
+//===--- 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().CPlusPlus11)
+ return;
+ Finder->addMatcher(
+ cxxThrowExpr(
+ stmt(forFunction(functionDecl(isNoThrow()).bind("func")))
+ )
+ .bind("throw"),
+ this);
+}
+
+bool isSimpleConstant(const Expr *E) {
+ return isa<CXXBoolLiteralExpr>(E);
+}
+
+bool isCatchedWithType(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 isCatchedInTry(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 isCatchedWithType(CaughtType, ThrownType);
+ }
+ return false;
+}
+
+bool isCatchedInFunction(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 && isCatchedInTry(Throw, Try)) {
+ return true;
+ }
+ for (const auto Parent: Context->getParents(node)) {
+ if (isCatchedInFunction(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;
+ }
+ }
+
+ if (isCatchedInFunction(Result.Context, Throw, Function, ast_type_traits::DynTypedNode::create(*Throw))) {
+ 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 no-throw 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
