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 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( "misc-throw-by-value-catch-by-reference"); + CheckFactories.registerCheck( + "misc-throw-with-noexcept"); CheckFactories.registerCheck( "misc-undelegated-constructor"); CheckFactories.registerCheck( 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(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() == Function) + return false; + + const auto *Try = node.get(); + 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("func"); + const auto *Throw = Result.Nodes.getNodeAs("throw"); + + llvm::SmallVector NoExceptRanges; + for (const auto *Redecl : Function->redecls()) { + const auto *Proto = Redecl->getType()->getAs(); + + 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("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: 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 + `_ check + + Flags ``throw`` statements in functions marked as no-throw. + - New `cert-dcl21-cpp `_ check @@ -71,7 +74,7 @@ `_ check Allow custom memory management functions to be considered as well. - + - New `misc-forwarding-reference-overload `_ 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 `_ 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 `_ check @@ -111,10 +114,10 @@ `_ 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 `_ and other safety 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/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: 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 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 int template_function() { + +template 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(); + template_function(); + template_class c1; + template_class 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; +}