Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -35,6 +35,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 @@ -43,6 +43,7 @@ #include "SuspiciousStringCompareCheck.h" #include "SwappedArgumentsCheck.h" #include "ThrowByValueCatchByReferenceCheck.h" +#include "ThrowWithNoexceptCheck.h" #include "UndelegatedConstructor.h" #include "UniqueptrResetReleaseCheck.h" #include "UnusedAliasDeclsCheck.h" @@ -122,6 +123,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 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,157 @@ +//===--- 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); +} + +// FIXME: Move to utils as it is used in multiple checks. +// Looks for the token matching the predicate and returns the range of the found +// token including trailing whitespace. +static SourceRange findToken(const SourceManager &Sources, LangOptions LangOpts, + SourceLocation StartLoc, SourceLocation EndLoc, + bool (*Pred)(const Token &)) { + if (StartLoc.isMacroID() || EndLoc.isMacroID()) + return SourceRange(); + FileID File = Sources.getFileID(Sources.getSpellingLoc(StartLoc)); + StringRef Buf = Sources.getBufferData(File); + const char *StartChar = Sources.getCharacterData(StartLoc); + Lexer Lex(StartLoc, LangOpts, StartChar, StartChar, Buf.end()); + Lex.SetCommentRetentionState(true); + Token Tok; + do { + Lex.LexFromRawLexer(Tok); + if (Pred(Tok)) { + Token NextTok; + Lex.LexFromRawLexer(NextTok); + return SourceRange(Tok.getLocation(), NextTok.getLocation()); + } + } while (Tok.isNot(tok::eof) && Tok.getLocation() < EndLoc); + + return SourceRange(); +} + +// Finds expression enclosed in parentheses, starting at StartLoc. +// Returns the range of the expression including trailing whitespace. +static SourceRange parenExprRange(const SourceManager &Sources, + LangOptions LangOpts, SourceLocation StartLoc, + SourceLocation EndLoc) { + if (StartLoc.isMacroID() || EndLoc.isMacroID()) + return SourceRange(); + FileID File = Sources.getFileID(Sources.getSpellingLoc(StartLoc)); + StringRef Buf = Sources.getBufferData(File); + const char *StartChar = Sources.getCharacterData(StartLoc); + Lexer Lex(StartLoc, LangOpts, StartChar, StartChar, Buf.end()); + Lex.SetCommentRetentionState(true); + Token Tok; + Lex.LexFromRawLexer(Tok); + assert(Tok.is(tok::l_paren)); + int OpenParenCount = 1; + do { + Lex.LexFromRawLexer(Tok); + if (Tok.is(tok::r_paren)) { + --OpenParenCount; + if (OpenParenCount == 0) { + Token NextTok; + Lex.LexFromRawLexer(NextTok); + return SourceRange(StartLoc, NextTok.getLocation()); + } + } else if (Tok.is(tok::l_paren)) { + ++OpenParenCount; + } + } while (Tok.isNot(tok::eof) && Tok.getLocation() < EndLoc); + + return SourceRange(); +} + +static SourceRange findNoExceptRange(const ASTContext &Context, + const SourceManager &Sources, + const FunctionDecl &Function) { + // FIXME: Support dynamic exception specification (e.g. throw()) + /* Find noexcept token */ + auto isKWNoexcept = [](const Token &Tok) { + return Tok.is(tok::raw_identifier) && Tok.getRawIdentifier() == "noexcept"; + }; + SourceRange NoExceptTokenRange = + findToken(Sources, Context.getLangOpts(), Function.getOuterLocStart(), + Function.getLocEnd(), isKWNoexcept); + + if (!NoExceptTokenRange.isValid()) + return SourceRange(); + + const auto *ProtoType = Function.getType()->getAs(); + const auto NoexceptSpec = ProtoType->getNoexceptSpec(Context); + if (NoexceptSpec != FunctionProtoType::NR_Nothrow) { + /* We have noexcept that doesn't evaluate to true, + * something strange or malformed. */ + return SourceRange(); + } else if (ProtoType->getNoexceptExpr() == nullptr) { + /* We have noexcept without the expression inside */ + return NoExceptTokenRange; + } + + /* Now we have to merge */ + SourceRange ExprRange = + parenExprRange(Sources, Context.getLangOpts(), + NoExceptTokenRange.getEnd(), Function.getLocEnd()); + + if (!ExprRange.isValid()) + return SourceRange(); + + return SourceRange(NoExceptTokenRange.getBegin(), ExprRange.getEnd()); +} + +void ThrowWithNoexceptCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Function = Result.Nodes.getNodeAs("func"); + const auto *Throw = Result.Nodes.getNodeAs("throw"); + diag(Throw->getLocStart(), "'throw' expression in a function declared with a " + "non-throwing exception specification"); + + llvm::SmallVector NoExceptRanges; + for (const auto *Redecl : Function->redecls()) { + SourceRange NoExceptRange = + findNoExceptRange(*Result.Context, *Result.SourceManager, *Redecl); + + 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; + } + } + + for (const auto &NoExceptRange : NoExceptRanges) { + // FIXME use DiagnosticIDs::Level::Note + diag(NoExceptRange.getBegin(), "In function declared no-throw here:") + << FixItHint::CreateRemoval(CharSourceRange::getCharRange( + NoExceptRange.getBegin(), NoExceptRange.getEnd())); + } +} + +} // namespace misc +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -63,9 +63,14 @@ explain them more clearly, and provide more accurate fix-its for the issues identified. The improvements since the 3.8 release include: +- New `misc-throw-with-noexcept + `_ check + + Flags ``throw`` statements in functions marked as no-throw. + - New Boost module containing checks for issues with Boost library. -- New `boost-use-to-string +- New `boost-use-to-string `_ check Finds usages of ``boost::lexical_cast`` and changes it to Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -83,6 +83,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,22 @@ +.. title:: clang-tidy - misc-throw-with-noexcept + +boost-use-to-string +=================== + +This check finds cases of using ``throw`` in a function declared as noexcept. +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 noexcept specifier as a fix. + + + .. code-block:: c++ + + void f() noexcept { + throw 42; + } + + // Will be changed to + void f() { + throw 42; + } Index: test/clang-tidy/misc-throw-with-noexcept.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-throw-with-noexcept.cpp @@ -0,0 +1,85 @@ +// RUN: %check_clang_tidy %s misc-throw-with-noexcept %t + +void f_throw_with_ne() noexcept(true) { + { + throw 5; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:24: warning: In function declared no-throw here: [misc-throw-with-noexcept] +// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept] +// CHECK-FIXES: void f_throw_with_ne() { + +void f_noexcept_false() noexcept(false) { + throw 5; +} + +void f_decl_only() noexcept; + +// Controversial example +void throw_and_catch() noexcept(true) { + try { + throw 5; + } catch (...) { + } +} +// CHECK-MESSAGES: :[[@LINE-6]]:24: warning: In function declared no-throw here: [misc-throw-with-noexcept] +// CHECK-MESSAGES: :[[@LINE-5]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept] +// CHECK-FIXES: void throw_and_catch() { + +void with_parens() noexcept((true)) { + throw 42; +} +// CHECK-MESSAGES: :[[@LINE-3]]:20: warning: In function declared no-throw here: [misc-throw-with-noexcept] +// CHECK-MESSAGES: :[[@LINE-3]]:2: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept] +// CHECK-FIXES: void with_parens() { + + +void with_parens_and_spaces() noexcept ( (true) ) { + throw 42; +} +// CHECK-MESSAGES: :[[@LINE-3]]:31: warning: In function declared no-throw here: [misc-throw-with-noexcept] +// CHECK-MESSAGES: :[[@LINE-3]]:2: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept] +// CHECK-FIXES: void with_parens_and_spaces() { + +class Class { + void InClass() const noexcept(true) { + throw 42; + } +}; +// CHECK-MESSAGES: :[[@LINE-4]]:24: warning: In function declared no-throw here: [misc-throw-with-noexcept] +// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept] +// CHECK-FIXES: void InClass() const { + + +void forward_declared() noexcept; +// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: In function declared no-throw here: [misc-throw-with-noexcept] +// CHECK-FIXES: void forward_declared() ; + +void forward_declared() noexcept { + throw 42; +} +// CHECK-MESSAGES: :[[@LINE-3]]:25: warning: In function declared no-throw here: [misc-throw-with-noexcept] +// CHECK-MESSAGES: :[[@LINE-3]]:2: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept] +// CHECK-FIXES: void forward_declared() { + +#define NOEXCEPT noexcept + +void with_macro() 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] + +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] +// FIXME expect a fix here + +void getFunction() noexcept { + struct X { + static void inner() + { + throw 42; + } + }; +}